[FFmpeg-devel] [PATCH]lavc/h264_ps: Check chroma_location limits

Michael Niedermayer michael at niedermayer.cc
Sat Mar 25 03:15:56 EET 2017


On Fri, Mar 24, 2017 at 10:16:42AM -0400, Ronald S. Bultje wrote:
[...]
> There is also the risk with the initial patch that compiling libavcodec
> against a newer libavutil but then runtime linking it with another (older)
> one would lead to crashes.

This should not be possible
the minor version should always be bumped when a new colorspace or
other is added. Building against that should not end up being linked
to older, it in fact could fail linking hard if it was from missing
symbols


> Clement's idea prevents that, but leads to loss
> of information. (I'm not saying the current approach of returning a
> potentially out-of-range value and expecting the end user to check/clip is
> any better, since some are likely to forget leading to crashes, but we need
> to think this through and make behaviour of the above code consistent with
> whatever we come up with.)

I think we are missing a clear documentation if out of the enum range
values are allowed in the field or not.

We could allow code to set any value in the field it likes and
require users to deal with this
or
Forbid code to ever set a value not supported by the current compile
time enum.  or Forbid code to ever set a value not supported by the
current link time enum.
Code reading still has to deal with new values as libs
can be updated and new values can be added to the enum.

I belive ubitux and carl implemented 2 of these options with their
patches

Each has its own advantages and disadvanatges

A: Export whatever is in the file as is

B: Never set a value that isnt known at the time a human updated it
   no possible conflicts with future adde enum values

C: Never have a value set that isnt supported by the libs functions
   like av_get_colorspace_name()


But what i really feel is relevant here is a different aspect
Andreas worked on cleaning this up previously, and carl and ubitux
now with these patches here continued this.
And we previously and now have near identical discussions.

I think we should either make a decission in FFmpeg on which way
it should be and document it. Or leave the people working on making
things consistent to do it as they prefer, they too should document it
of course.

Repeatly starting a discussion when some patch comes up on a subject
but then failing to really reach a decission and failing to document
it and repeating the cycle over and over is not good


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The educated differ from the uneducated as much as the living from the
dead. -- Aristotle 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170325/43307dab/attachment.sig>


More information about the ffmpeg-devel mailing list