[FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 24, 2017 at 10:17 PM, Nicolas George <george at nsup.org> wrote:
> Le tridi 3 brumaire, an CCXXVI, Jan Ekstrom a écrit :
> I do not consider this a public API change because 0 was never
> documented as a valid return value for a read_packet callback, while
> AVERROR_EOF has been around for a long time.
>

You might be very true, but funny enough I just checked the AVIO
example doc/examples/avio_reading.c, and the read function there does
not seem to utilize AVERROR_EOF either :) .

> But my goal is not to break existing code on purpose. Fortunately, there
> is a rather simple workaround, I will try to implement it tomorrow.
> Still, lacking a simple test case, I will not be able to test it.
>

Just for your information, I decided to call people out a bit on the
IRC channel to see what people would prefer, and it seems like James,
BtbN and Martin voiced their opinion that they would like a flag that
would let you enable the 0 != EOF behavior. Additionally, a warning
would be given out if zero is returned and the flag is unset. Quote
follows:

> <+JEEB> can anyone else chime on the EOF thing, if we want to
>         either A) leave things as they are and just break old
>         clients with an API behavior change B) add an option for
>         the new behavior , or C) revert the 0 != EOF thing
> <+JEEB> because while those people that are on IRC have most
>         likely adapted their stuff, we will have quite a bit of
>         breakage due to this :P
> <+JEEB> I'd probably go for B but I'm not sure how many places I'd
>         have to poke to add the option
> < jamrial> maybe a temporary option to recover the old behavior
>            should be added for the usual period of ~2 years, and a
>            notice about it being removed eventually added
> < jamrial> basically, deprecating the old behavior and leaving a
>            compat mode for it
> <+JEEB> yes
> < jamrial> library users will nontheless have to update their code
>            to enable said option
> < jamrial> then again to remove it two years from now :p
> < jamrial> so dunno
> < jamrial> is this change, unintended breakage aside, a good one?
>            because if it doesn't really bring any worthwile
>            benefit maybe we should just revert it
> <+JEEB> the requirement for the change was zero-byte UDP packets
>         which seem to be valid. originally the code was added into
>         the UDP lavf module, but then it was requested to be moved
>         into lavf general
> <+JEEB> which thus broke the API behavior
> <@nevcairiel> jamrial: i have always questioned the usefulness of
>               the change, but apparently there is obscure  crazy
>               things that might use it
> <+JEEB> yes, most users don't find that stuff on their networks
> <@nevcairiel> and re: flag, if anything it should be inverted, or
>               its still an API break =p
> <+JEEB> well flag to have the NEW behavior
> <+JEEB> so that the default behavior is the old one
> <+JEEB> that's what I meant
> < jamrial> yeah, probably
> <@BtbN> It should behave as it used to, which is imo a bit broken,
>         with some flag somewhere to flip it to sane behaviour
> <+JEEB> BtbN: agreed
> <@BtbN> And it should throw a depercation warning if the flag is
>         unset
> <@wbs> alternatively, only throw a warning if the flag is unset
>        _and_ you return 0?
> <@wbs> then you can write code that uses AVERROR_EOF for older
>        lavf versions without any ifdefs for the new flag, while
>        still running without warnings on the new lavf as well

Any opinions?


Jan