On 21.07.2014 21:23, Michael Niedermayer wrote:
On Mon, Jul 21, 2014 at 08:05:05PM +0200, Andreas Cadhalpun wrote:
On 21.07.2014 00:48, Michael Niedermayer wrote:
On Sun, Jul 20, 2014 at 10:43:30PM +0200, Andreas Cadhalpun wrote:
On 12.06.2014 08:42, Christophe Gisquet wrote:
Hi,
2014-06-11 21:29 GMT+02:00 Michael Niedermayer <michae...@gmx.at>:
-#define FF_INPUT_BUFFER_PADDING_SIZE 16
+#define FF_INPUT_BUFFER_PADDING_SIZE 32
So, following the discussion on how API users may be affected by this
change, does that need an API bump?
One effect of this change is that now mplayer2 fails to build,
because it uses this check:
#if MP_INPUT_BUFFER_PADDING_SIZE < FF_INPUT_BUFFER_PADDING_SIZE
#error MP_INPUT_BUFFER_PADDING_SIZE is too small!
#endif
MP_INPUT_BUFFER_PADDING_SIZE is defined as 16. Increasing that to 32
allows building mplayer2 again.
So I think an API bump wouldn't have been a bad idea.
I think a soname bump would cause alot more work for everyone
That's for sure. I didn't mean a soversion bump for this change
alone, but rather wait with this change until a soversion bump is
necessary due to a larger API change.
It's just my expectation that if the API (i.e. major soversion)
doesn't change, programs that built successfully with an older
version will continue to build fine.
This change breaks that expectation.
yes though the failure is due to an app explicitly checking
FF_INPUT_BUFFER_PADDING_SIZE to be within a specific range
our API did not gurantee that it would be in that range.
also FF_ is the prefix for internal stuff AV* for public, arguably
it probably should have been AV_ but it isnt
I agree, it's rather unorthodox to error out if
FF_INPUT_BUFFER_PADDING_SIZE is too large.
but lets assume we did bump soname and changed it to 32 after that
what with the old API/ABI ? it needs 32 too in the exact same corner
cases that the new API/ABI needs it. But all apps that are build
against the old AND which use these corner cases are then buggy.
that doesnt sound better to me
I don't know how severe these corner case bugs have been.
Would it have been a big problem to not fix them until the next
soversion bump?
iam not sure i understand you here
mplayer2 didnt pad by enough (assuming it can use that corner case)
and ffmpeg didnt pad by enough either
mplayer2 should be using 32 even if ffmpeg doesnt, you want to pad
by enough (that is again assuming it uses a code path that needs it)
so i really dont see who would be helped by leaving
FF_INPUT_BUFFER_PADDING_SIZE at 16 (unless all code really needs just
that)
about severity of the corner cases, it can cause a crash.
And i do not have a list of which functions need 32byte padding and
which dont. One case is fate-vsynth3-rgb or more precissely the
optimized colorspace convertion/scaling in it
That sounds severe enough to warrant an immediate fix.
I think the soversion should have been increased, when the first
function needing 32 byte padding was added, together with increasing the
FF_INPUT_BUFFER_PADDING_SIZE.
In any case mentioning such a change in APIChanges would be good.
ok, done
Thanks.
Thanks
PS: i understand your point about this but i really think changing
the FF_INPUT_BUFFER_PADDING_SIZE was better and less work for everyone
than delaying it and leaving the bugs open.
OK, then it's fine.
Best regards,
Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
