Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-07-18 Thread Alessandro Ghedini
Control: tags -1 fixed-upstream

On Mon, Jun 29, 2015 at 06:01:04pm +0200, Guillem Jover wrote:
 Package: mpv
 Version: 0.9.2-1+ffmpeg
 Severity: normal
 
 Hi!
 
 [ First of all, thanks for providing a ffmpeg version of the package,
   there's quite some media that does not play correctly with libav. ]
 
 The mpv program emits the following warning on startup:
 
 ,---
 Warning: mpv was compiled against a different version of ffmpeg than the 
 shared
 library it is linked against. This can expose subtle ABI compatibility issues
 and can lead to misbehavior and crashes.
 `---
 
 Which, to me points out there's a problem somewhere. Either:
 
  * the warning is bogus, and
- mpv should be silenced in common/av_log.c:print_libav_versions, or

Upstream removed the warning now, see [0].

Cheers

[0] 
https://github.com/mpv-player/mpv/commit/5594379b27f3c19a475b83a001a1cd96946c


signature.asc
Description: Digital signature


Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-07-07 Thread Alessandro Ghedini
Control: forwarded -1 https://github.com/mpv-player/mpv/issues/2110

Sorry for the delay.

On mer, lug 01, 2015 at 10:35:13 +0200, Andreas Cadhalpun wrote:
 Hi Guillem,
 
 On 30.06.2015 23:14, Andreas Cadhalpun wrote:
  On 30.06.2015 21:40, Guillem Jover wrote:
  Perhaps, but the comment at
  http://sources.debian.net/src/mpv/0.9.2-1%2Bffmpeg/common/av_log.c/#L229
  was not very soothing, so that was one additional reason for the bug. :)
  
  Hmm, this comment is indeed a bit worrisome.
  But I'm not sure I understand the reasoning given there. As far as I know
  the accessor functions are only necessary for API that is not present
  in Libav (in order to be ABI compatible after Libav merges, as the comment
  says). So whether or not one uses the accessors shouldn't make a difference
  for compatibility with Libav.
 
 I've investigated this a bit more and it seems that the only potentially
 problematic FFmpeg-only API used by mpv is AVFrame-metadata, which mpv
 accesses via av_frame_get_metadata. [1]
 Thus I think the comment is just wrong and the warning should be removed.

If I understand the comment correctly, the problem isn't so much ffmpeg-only
APIs like -metadata, but the fact that mpv doesn't use accessor functions for
struct fields provided by ffmpeg to maintain ABI compatibility because libav
doesn't have those functions (but it still has the structs).

IMO the warning can be removed in the Debian packages because it's fairly
annoying and mostly useless (it's not like Debian users can do anything about
it besides recompiling the package), but maybe we should better investigate
this alleged ABI compatibility problem in ffmpeg.

For example the upstream-tracker thingy shows several ABI changes introduced in
ffmpeg 2.5 (e.g. new fields in the middle of structs) without a SONAME bump:
http://upstream.rosalinux.ru/versions/ffmpeg.html

The mpv Debian package could also start using the accessors that ffmpeg
provides but only when built with ffmpeg (which should hopefully become the
default soonish), but tracking down where these accessors should be used is
probably going to be a bit of a pain.

Cheers


signature.asc
Description: Digital signature


Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-07-07 Thread Andreas Cadhalpun
Hi Alessandro,

On 07.07.2015 17:26, Alessandro Ghedini wrote:
 On mer, lug 01, 2015 at 10:35:13 +0200, Andreas Cadhalpun wrote:
 I've investigated this a bit more and it seems that the only potentially
 problematic FFmpeg-only API used by mpv is AVFrame-metadata, which mpv
 accesses via av_frame_get_metadata. [1]
 Thus I think the comment is just wrong and the warning should be removed.
 
 If I understand the comment correctly, the problem isn't so much ffmpeg-only
 APIs like -metadata, but the fact that mpv doesn't use accessor functions for
 struct fields provided by ffmpeg to maintain ABI compatibility because libav
 doesn't have those functions (but it still has the structs).

That wouldn't make much sense. If it is safe to access a struct member in Libav,
it is also safe to access it in FFmpeg. In that case there is no need for
accessor functions. These are either also necessary (and present) in Libav,
or it is an FFmpeg-only API like AVFrame-metadata.

 IMO the warning can be removed in the Debian packages because it's fairly
 annoying and mostly useless (it's not like Debian users can do anything about
 it besides recompiling the package), but maybe we should better investigate
 this alleged ABI compatibility problem in ffmpeg.

These changes in private ABI are well-known and caused by merges from Libav:
In FFmpeg some structs have more members than in Libav. If there is a new
member added at the end of the struct in Libav, it is inserted in the middle
of the struct in FFmpeg. Thus all FFmpeg-only struct members must never be
accessed directly. (This is documented very clearly.)

 For example the upstream-tracker thingy shows several ABI changes introduced 
 in
 ffmpeg 2.5 (e.g. new fields in the middle of structs) without a SONAME bump:
 http://upstream.rosalinux.ru/versions/ffmpeg.html

Yes, e.g. the framerate and initial_padding members were added to the end
of AVCodecContext in Libav and merged into FFmpeg between 2.4 and 2.5.

 The mpv Debian package could also start using the accessors that ffmpeg
 provides but only when built with ffmpeg

It already does exactly that! ;)

 (which should hopefully become the
 default soonish), but tracking down where these accessors should be used is
 probably going to be a bit of a pain.

Since this only affects FFmpeg-only struct members and mpv is also compatible
with Libav, thus using ifdeffery for FFmpeg-only functionality, finding the
potentially problematic case(s) is not that hard.
There is only AVFrame-metadata, which is accessed correctly.

Best regards,
Andreas


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-07-01 Thread Andreas Cadhalpun
Hi Guillem,

On 30.06.2015 23:14, Andreas Cadhalpun wrote:
 On 30.06.2015 21:40, Guillem Jover wrote:
 Perhaps, but the comment at
 http://sources.debian.net/src/mpv/0.9.2-1%2Bffmpeg/common/av_log.c/#L229
 was not very soothing, so that was one additional reason for the bug. :)
 
 Hmm, this comment is indeed a bit worrisome.
 But I'm not sure I understand the reasoning given there. As far as I know
 the accessor functions are only necessary for API that is not present
 in Libav (in order to be ABI compatible after Libav merges, as the comment
 says). So whether or not one uses the accessors shouldn't make a difference
 for compatibility with Libav.

I've investigated this a bit more and it seems that the only potentially
problematic FFmpeg-only API used by mpv is AVFrame-metadata, which mpv
accesses via av_frame_get_metadata. [1]
Thus I think the comment is just wrong and the warning should be removed.

Best regards,
Andreas


1: 
https://sources.debian.net/src/mpv/0.9.2-1%2Bffmpeg/video/filter/vf_lavfi.c/#L309


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-06-30 Thread Guillem Jover
Hey!

On Mon, 2015-06-29 at 21:45:26 +0200, Andreas Cadhalpun wrote:
 (We plan to transition all packages from Libav to FFmpeg soon, see [1].)

Yeah, I'm aware, eagerly waiting it!

 On 29.06.2015 18:01, Guillem Jover wrote:
  The mpv program emits the following warning on startup:
  
  ,---
  Warning: mpv was compiled against a different version of ffmpeg than the 
  shared
  library it is linked against. This can expose subtle ABI compatibility 
  issues
  and can lead to misbehavior and crashes.
  `---
 
 I guess this warning is meant as sanity check for those compiling mpv 
 themselves.
 (The ffmpeg binary also checks for matching compile-time and run-time 
 libraries.)
 These warnings aren't very useful in a distribution, where package 
 dependencies
 ensure ABI compatibility.

Perhaps, but the comment at
http://sources.debian.net/src/mpv/0.9.2-1%2Bffmpeg/common/av_log.c/#L229
was not very soothing, so that was one additional reason for the bug. :)

  Which, to me points out there's a problem somewhere. Either:
  
   * the warning is bogus, and
 - mpv should be silenced in common/av_log.c:print_libav_versions, or
 
 If the warning bothers you, this would be the way to go.

It's not so much that it bothers me; as I've mentioned, either the
warning is valid and there's a potential ABI problem, or it is not
and then emitting an alarming message that the user can hardly fix
on each invocation might be slightly taxing, it is at least a sure
way to train users to ignore valid ones. :)

 - ffmpeg should not be exposing the minor versions in its macros
   and *_version() functions, so that such warnings do not trigger, or
 
 The minor versions are used to e.g. indicate new features, so that programs
 can do exact compile-time checks. Not exposing them would break that.

How does the libav project handle this? I don't remember seeing such
warning with an mpv compiled against that.

   * the warning is valid, and
 - mpv needs a strict versioned dependency, or
 
 The dependencies are calculated by the symbols mechanism.

Sure.

 No stricter dependencies should be necessary.

In principle, but I was going by the comment on that source file. Sorry,
I should have provided a direct link initially.

 - mpv very unfortunately needs to statically link against ffmpeg, or
 
 Not that, please!

Well, I abhor such kind of solutions with a passion, but if there's no
better option…

Thanks,
Guillem


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-06-30 Thread Andreas Cadhalpun
Hi Guillem,

On 30.06.2015 21:40, Guillem Jover wrote:
 On Mon, 2015-06-29 at 21:45:26 +0200, Andreas Cadhalpun wrote:
 (We plan to transition all packages from Libav to FFmpeg soon, see [1].)
 
 Yeah, I'm aware, eagerly waiting it!

:)

 On 29.06.2015 18:01, Guillem Jover wrote:
 I guess this warning is meant as sanity check for those compiling mpv 
 themselves.
 (The ffmpeg binary also checks for matching compile-time and run-time 
 libraries.)
 These warnings aren't very useful in a distribution, where package 
 dependencies
 ensure ABI compatibility.
 
 Perhaps, but the comment at
 http://sources.debian.net/src/mpv/0.9.2-1%2Bffmpeg/common/av_log.c/#L229
 was not very soothing, so that was one additional reason for the bug. :)

Hmm, this comment is indeed a bit worrisome.
But I'm not sure I understand the reasoning given there. As far as I know
the accessor functions are only necessary for API that is not present
in Libav (in order to be ABI compatible after Libav merges, as the comment
says). So whether or not one uses the accessors shouldn't make a difference
for compatibility with Libav.

 Which, to me points out there's a problem somewhere. Either:

  * the warning is bogus, and
- mpv should be silenced in common/av_log.c:print_libav_versions, or

 If the warning bothers you, this would be the way to go.
 
 It's not so much that it bothers me; as I've mentioned, either the
 warning is valid and there's a potential ABI problem, or it is not
 and then emitting an alarming message that the user can hardly fix
 on each invocation might be slightly taxing, it is at least a sure
 way to train users to ignore valid ones. :)

Indeed. In this case clarifying with upstream what this warning means,
i.e. which accessors are not used, would be good.

- ffmpeg should not be exposing the minor versions in its macros
  and *_version() functions, so that such warnings do not trigger, or

 The minor versions are used to e.g. indicate new features, so that programs
 can do exact compile-time checks. Not exposing them would break that.
 
 How does the libav project handle this?

Identically.

 I don't remember seeing such warning with an mpv compiled against that.

The Libav project has only made new releases about once a year, usually
with a SONAME bump, so that mpv got recompiled.
A binNMU would make the warning go away.

  * the warning is valid, and
- mpv needs a strict versioned dependency, or

 The dependencies are calculated by the symbols mechanism.
 
 Sure.
 
 No stricter dependencies should be necessary.
 
 In principle, but I was going by the comment on that source file. Sorry,
 I should have provided a direct link initially.

Yes, that comment is important.

- mpv very unfortunately needs to statically link against ffmpeg, or

 Not that, please!
 
 Well, I abhor such kind of solutions with a passion, but if there's no
 better option…

The better solution would be to use the API as documented, i.e. with the
accessor functions.
In particular because I don't see a reason not to do that.

Best regards,
Andreas


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-06-29 Thread Guillem Jover
Package: mpv
Version: 0.9.2-1+ffmpeg
Severity: normal

Hi!

[ First of all, thanks for providing a ffmpeg version of the package,
  there's quite some media that does not play correctly with libav. ]

The mpv program emits the following warning on startup:

,---
Warning: mpv was compiled against a different version of ffmpeg than the shared
library it is linked against. This can expose subtle ABI compatibility issues
and can lead to misbehavior and crashes.
`---

Which, to me points out there's a problem somewhere. Either:

 * the warning is bogus, and
   - mpv should be silenced in common/av_log.c:print_libav_versions, or
   - ffmpeg should not be exposing the minor versions in its macros
 and *_version() functions, so that such warnings do not trigger, or
 * the warning is valid, and
   - mpv needs a strict versioned dependency, or
   - mpv very unfortunately needs to statically link against ffmpeg, or
 * something else?

I'm filing it here, because it's not clear to me where the real
problem lies. Please reassign as appropriate if need be.

Thanks,
Guillem


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#790446: mpv: Warning about mismatch between build and run-time ffmpeg libraries

2015-06-29 Thread Andreas Cadhalpun
Hi Guillem,

On 29.06.2015 18:01, Guillem Jover wrote:
 [ First of all, thanks for providing a ffmpeg version of the package,
   there's quite some media that does not play correctly with libav. ]

Credits for that go to Alessandro.
(We plan to transition all packages from Libav to FFmpeg soon, see [1].)

 The mpv program emits the following warning on startup:
 
 ,---
 Warning: mpv was compiled against a different version of ffmpeg than the 
 shared
 library it is linked against. This can expose subtle ABI compatibility issues
 and can lead to misbehavior and crashes.
 `---

I guess this warning is meant as sanity check for those compiling mpv 
themselves.
(The ffmpeg binary also checks for matching compile-time and run-time 
libraries.)
These warnings aren't very useful in a distribution, where package dependencies
ensure ABI compatibility.

 Which, to me points out there's a problem somewhere. Either:
 
  * the warning is bogus, and
- mpv should be silenced in common/av_log.c:print_libav_versions, or

If the warning bothers you, this would be the way to go.

- ffmpeg should not be exposing the minor versions in its macros
  and *_version() functions, so that such warnings do not trigger, or

The minor versions are used to e.g. indicate new features, so that programs
can do exact compile-time checks. Not exposing them would break that.

  * the warning is valid, and
- mpv needs a strict versioned dependency, or

The dependencies are calculated by the symbols mechanism. No stricter
dependencies should be necessary.

- mpv very unfortunately needs to statically link against ffmpeg, or

Not that, please!

  * something else?
 
 I'm filing it here, because it's not clear to me where the real
 problem lies. Please reassign as appropriate if need be.

I think you were right to file this bug against mpv.

Best regards,
Andreas


1: https://wiki.debian.org/Debate/libav-provider/ffmpeg


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org