[MPlayer-users] Build regression on FreeBSD

[Thomas Zander]

Hi,

porting an up-to-date snapshot to FreeBSD I noticed two build
regressions and want to kindly ask for resolving those upstream:

1) Makefile (at root dir)
codec-cfg.c now includes <vdpau/vdpau.h>
On FreeBSD this file ends up in LOCALBASE (by default
/usr/local/include) which is not included in the default compiler
search path, so in its build target
codec-cfg$(EXESUF) .... the compile command
$(HOST_CC) $(HOSTCFLAGS) -o $@ $<
is not sufficient. The include path for localbase needs to be added.

2) stream/stream_vcd.c:
when building with libcdio, vcd_read_fbsd.h is no longer included,
which, in turn, causes for sys/cdrio.h not to be included.
Subsequently CDRIOCSETBLOCKSIZE is an undefined symbol and
compilation breaks. Including sys/cdrio.h in stream/stream_vcd.c
resolves the problem and the build completes.

Best regards
Riggs

[Ingo Brückl]

Post by Thomas Zander
when building with libcdio, vcd_read_fbsd.h is no longer included,
which, in turn, causes for sys/cdrio.h not to be included.
Subsequently CDRIOCSETBLOCKSIZE is an undefined symbol and
compilation breaks. Including sys/cdrio.h in stream/stream_vcd.c
resolves the problem and the build completes.

I suppose we don't need the ioctl CDRIOCSETBLOCKSIZE in case of libcdio,
do we? When libcdio is used, it should take care and the call should be
unnecessary.

Easy to fix. (Are you able to test with a VCD?)

Ingo

[Alexander Strasser]

Hi Thomas,

AFAICT there are multiple problems at work, that make a
quick solution rather involved or hackish. That's why I
have Cc'ed MPlayer development mailing list.

Post by Thomas Zander
porting an up-to-date snapshot to FreeBSD I noticed two build
1) Makefile (at root dir)
codec-cfg.c now includes <vdpau/vdpau.h>
On FreeBSD this file ends up in LOCALBASE (by default
/usr/local/include) which is not included in the default compiler
search path, so in its build target
codec-cfg$(EXESUF) .... the compile command
is not sufficient. The include path for localbase needs to be added.

I guess r37454 introduced this problem, by including vdpau.h in
our img_format.h header. It cannot be reverted because that would
probably break the build with future FFmpeg versions.

The problem is if we want to use external stuff, which cannot be
forward declared, in img_format.h we need to add additional flags
to HOSTCFLAGS as well.

Furthermore it is interesing that vdpau gets detected at all on
your system. I suspect this is because of manual addition to CFLAGS
or because of modification by previous configure tests.

I might be missing a few things, but all in all I do not see why
we need the structure vdpau_frame_data in codec-cfg.c source. So
maybe the structure should be defined in a different header?

All in all I see no really easy way to set HOSTCFLAGS correctly.
Additionally I am not sure if the work is justified, because if
I am not mistaken above it is not needed in codec-cfg.

[...]

Alexander

[Roberto Togni]

On Sun, 20 Dec 2015 22:05:01 +0100

Post by Alexander Strasser
Hi Thomas,
AFAICT there are multiple problems at work, that make a
quick solution rather involved or hackish. That's why I
have Cc'ed MPlayer development mailing list.

Post by Thomas Zander
porting an up-to-date snapshot to FreeBSD I noticed two build
1) Makefile (at root dir)
codec-cfg.c now includes <vdpau/vdpau.h>
On FreeBSD this file ends up in LOCALBASE (by default
/usr/local/include) which is not included in the default compiler
search path, so in its build target
codec-cfg$(EXESUF) .... the compile command
is not sufficient. The include path for localbase needs to be added.

I guess r37454 introduced this problem, by including vdpau.h in
our img_format.h header. It cannot be reverted because that would
probably break the build with future FFmpeg versions.
The problem is if we want to use external stuff, which cannot be
forward declared, in img_format.h we need to add additional flags
to HOSTCFLAGS as well.
Furthermore it is interesing that vdpau gets detected at all on
your system. I suspect this is because of manual addition to CFLAGS
or because of modification by previous configure tests.
I might be missing a few things, but all in all I do not see why
we need the structure vdpau_frame_data in codec-cfg.c source. So
maybe the structure should be defined in a different header?
All in all I see no really easy way to set HOSTCFLAGS correctly.
Additionally I am not sure if the work is justified, because if
I am not mistaken above it is not needed in codec-cfg.

Do you see the failure only in codec-cfg.c or also in the other files
including vdpau.h (vd_ffmpeg and vo_vdpau)?

If it's only codec-cgg.c, the easiest way is to move the structure
somewhere else; I guess it was put there only because img_format.h is
included both in vd_ffmpeg and vo_vdpau.
The struct has always been there, but it was not using any field
requiring external vdpau headers before.


If you can confirm that the problem is only with codec-cfg.c I'll just
move the struct and the include; best candidates are vo_out.h, vd.h or a
dedicated header.


Ciao,
Roberto

[Alexander Strasser]

Post by Roberto Togni
On Sun, 20 Dec 2015 22:05:01 +0100

Post by Alexander Strasser
AFAICT there are multiple problems at work, that make a
quick solution rather involved or hackish. That's why I
have Cc'ed MPlayer development mailing list.

Post by Thomas Zander
porting an up-to-date snapshot to FreeBSD I noticed two build
1) Makefile (at root dir)
codec-cfg.c now includes <vdpau/vdpau.h>
On FreeBSD this file ends up in LOCALBASE (by default
/usr/local/include) which is not included in the default compiler
search path, so in its build target
codec-cfg$(EXESUF) .... the compile command
is not sufficient. The include path for localbase needs to be added.

I guess r37454 introduced this problem, by including vdpau.h in
our img_format.h header. It cannot be reverted because that would
probably break the build with future FFmpeg versions.
The problem is if we want to use external stuff, which cannot be
forward declared, in img_format.h we need to add additional flags
to HOSTCFLAGS as well.
Furthermore it is interesing that vdpau gets detected at all on
your system. I suspect this is because of manual addition to CFLAGS
or because of modification by previous configure tests.
I might be missing a few things, but all in all I do not see why
we need the structure vdpau_frame_data in codec-cfg.c source. So
maybe the structure should be defined in a different header?
All in all I see no really easy way to set HOSTCFLAGS correctly.
Additionally I am not sure if the work is justified, because if
I am not mistaken above it is not needed in codec-cfg.

Do you see the failure only in codec-cfg.c or also in the other files
including vdpau.h (vd_ffmpeg and vo_vdpau)?
If it's only codec-cgg.c, the easiest way is to move the structure
somewhere else; I guess it was put there only because img_format.h is
included both in vd_ffmpeg and vo_vdpau.
The struct has always been there, but it was not using any field
requiring external vdpau headers before.

If I am not mistaken this is the changeset from Thomas:

http://svnweb.freebsd.org/ports?view=revision&revision=404026

This is the change to the Makefile patch which extends HOSTCFLAGS only:

http://svnweb.freebsd.org/ports/head/multimedia/mplayer/files/patch-Makefile?r1=404026&r2=404025&pathrev=404026

There are more changes to configure and MPlayer's Makefile
in those patch files, I did not check all of them. Maybe Thomas
can clarify?

Post by Roberto Togni
If you can confirm that the problem is only with codec-cfg.c I'll just
move the struct and the include; best candidates are vo_out.h, vd.h or a
dedicated header.

I would favour this solutions too.


Alexander

[Alexander Strasser]

Hi Roberto!
[...]

Post by Alexander Strasser

Post by Roberto Togni

Post by Alexander Strasser
I might be missing a few things, but all in all I do not see why
we need the structure vdpau_frame_data in codec-cfg.c source. So
maybe the structure should be defined in a different header?
All in all I see no really easy way to set HOSTCFLAGS correctly.
Additionally I am not sure if the work is justified, because if
I am not mistaken above it is not needed in codec-cfg.

Do you see the failure only in codec-cfg.c or also in the other files
including vdpau.h (vd_ffmpeg and vo_vdpau)?
If it's only codec-cgg.c, the easiest way is to move the structure
somewhere else; I guess it was put there only because img_format.h is
included both in vd_ffmpeg and vo_vdpau.
The struct has always been there, but it was not using any field
requiring external vdpau headers before.

http://svnweb.freebsd.org/ports?view=revision&revision=404026
http://svnweb.freebsd.org/ports/head/multimedia/mplayer/files/patch-Makefile?r1=404026&r2=404025&pathrev=404026
There are more changes to configure and MPlayer's Makefile
in those patch files, I did not check all of them. Maybe Thomas
can clarify?

Post by Roberto Togni
If you can confirm that the problem is only with codec-cfg.c I'll just
move the struct and the include; best candidates are vo_out.h, vd.h or a
dedicated header.

I would favour this solutions too.

If I didn't miss something, this was also reported by a non-BSD user on
our bug tracker:

https://trac.mplayerhq.hu/ticket/2265

Tell me if I should give it a try. I just don't want to end up doing
duplicate work.


Alexander

[Ingo Brückl]

Post by Ingo Brückl

Post by Thomas Zander
when building with libcdio, vcd_read_fbsd.h is no longer included,
which, in turn, causes for sys/cdrio.h not to be included.
Subsequently CDRIOCSETBLOCKSIZE is an undefined symbol and
compilation breaks. Including sys/cdrio.h in stream/stream_vcd.c
resolves the problem and the build completes.

I suppose we don't need the ioctl CDRIOCSETBLOCKSIZE in case of libcdio,
do we? When libcdio is used, it should take care and the call should be
unnecessary.

I found the ioctl CDRIOCSETBLOCKSIZE call in libcdio so we should go with the
attached patch.

Ingo

[Ingo Brückl]

Post by Thomas Zander
porting an up-to-date snapshot to FreeBSD I noticed two build

This one is fixed.

Ingo