[FFmpeg-devel] [PATCH] oggparseskeleton: return 0 on EOS packet

Reimar Döffinger Reimar.Doeffinger at gmx.de
Mon Jan 6 23:22:27 CET 2014


On 06.01.2014, at 22:07, James Almer <jamrial at gmail.com> wrote:
> On 06/01/14 6:25 AM, Reimar Döffinger wrote:
>> On 06.01.2014, at 08:25, James Almer <jamrial at gmail.com> wrote:
>>> It's the last packet
>> 
>> What does this fix?
>> I'd rather not rely on EOS being set correctly unless there is an actual need.
>> (not to mention that it's always helpful if the reason for the change is properly described in the log, so one can figure out what to test if it breaks something else and needs to be solved differently).
> 
> I have a theora+skeleton file (with EOS set correctly) that spams a lot of "Broken file, 
> keyframe not correctly marked." and a vp8+skeleton that spams "read_timestamp() failed in 
> the middle" errors when using seek-test without this patch.

I'd like to really understand the why, this sounds like it might just hide the real bug/issue?

> With it the amount of warnings in the theora file goes down considerably, and the errors 
> in the vp8 file disappear.
> The output of seek-test also changes.

That would be more convincing if you said it is fixed, "changes" is not very convincing?

> That aside, returning 0 is correct since as i said the EOS packet is the last one.

I am fairly certain that one can trivially create a file where it isn't.

> The behavior of broken files shouldn't change. It will still return -1 if the packet size 
> is < 8, or 1 if none of the tests below the EOS one succeed (Which i think is wrong and 
> should be changed to 0 or -1 since anything that's not "fishead", "fisbone" or EOS is 
> invalid).

Maybe I misunderstood you, but I meant a file corruption where a packet in the middle ends up looking like a EOS one.

> If you prefer i could make it so it returns -1 if the EOS packet size is > 0, which is the 
> only other scenario that's not being considered.

Hm, maybe I'd have to first figure out what the different return values actually mean.
Just intuitively I'd think that whether and where we have an EOS should impact behaviour only minimally - preferably not at all.


More information about the ffmpeg-devel mailing list