[FFmpeg-devel] [PATCH 2/2] avformat/movenc: parse h264 packets to build Sync Sample and Recovery Point tables

[James Almer]

On 7/30/2021 8:33 AM, Michael Niedermayer wrote:
> On Thu, Jul 29, 2021 at 03:09:03PM -0300, James Almer wrote:
>> On 7/29/2021 2:58 PM, Michael Niedermayer wrote:
>>> On Tue, Jul 27, 2021 at 10:08:20AM -0300, James Almer wrote:
>>>> Since we can't blindly trust the keyframe flag in packets and assume its
>>>> contents are a valid Sync Sample, do some basic bitstream parsing to build the
>>>> Sync Sample table in addition to a Random Access Recovery Point table.
>>>>
>>>> Suggested-by: ffmpeg at fb.com
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>>    libavformat/movenc.c         | 125 +++++++++++++++++++++++++++++++++--
>>>>    libavformat/movenc.h         |   1 +
>>>>    tests/ref/lavf-fate/h264.mp4 |   6 +-
>>>>    3 files changed, 123 insertions(+), 9 deletions(-)
>>>
> 
> A.
>>> Will this allow a user to mark a subset of keyframes as random
>>> access points ?
>>> A user may want seeking to preferably happen to a scene start and not a
>>> frame before
>>>
> 
> B.
>>> Will this allow a user to mark a damaged keyframe as such ?
>>> A frame might in fact have been a keyframe and maybe the only in the file
>>> but maybe was damaged so a parser might fail to detect it.
>>
>> This code will ensure all packets with an IDR picture will be listed in the
>> Sync Sample table, and all packets with a Recovery Point SEI message will be
>> listed in the Random Access Recovery Point table.
>> Whatever is signaled in packets (like the keyframe flag) is ignored to
>> prevent creating non-spec compliant output.
> 
> The file in case B will be non compliant, it is damaged data, it cannot
> be muxed in a compliant way. If we support muxing damaged data then
> parsing that data as a way to unconditionally override the user is
> problematic.
> If you try to interpret damaged data the result is undefined, the spec
> does not tell you how to interpret it and 2 parsers can disagree
> if a damaged frame is a keyframe.

If you don't mark a packet as a Sync Sample, even if it contains damaged 
data, the file is still compliant. The point here is to avoid filling 
the Sync Sample table with bogus entries.

> 
> Lets just as 2 examples consider multiple slices some being parsed as IDR
> some as non IDR, so what is that ? or what if some packet reodering or
> duplication causes parts of a IDR and non IDR frame to be mixed.
> where is the IDR frame in that case ? 2 parsers can disagree

The AVCodecParserContext will tag a packet with an IDR slice as 
keyframe, and then disregard anything else it may find. I made the code 
i wrote here do the same.

> still a decoder can with some luck start decoding from such a
> point i would guess. But does the user want this marked as a keyframe ?
> maybe yes, maybe no. I think taking the users only way to set the keyframe
> flag away is a step backward

Look at MPEG2 parsing and others in this container. It's the same 
scenario. Did any of these concerns show up back then?
What I'm doing here for h264 is the same thing we did for those, to 
ensure we don't create non spec compliant files because of damaged or 
mistagged input, or a careless user.

Letting the user mark whatever they want as a Sync Sample on a container 
where it's explicit what should and should not be such is not good 
practice. If you want that, you can use Matroska and other formats where 
keyframe tagging is apparently not strict.

Now, i want to point out that I wrote this parsing code here because my 
previous attempt at stopping the AVCodecParserContext from being too 
liberal marking things as keyframes was rejected *precisely* because it 
would let the user create non compliant output in mp4. And now you don't 
want this one either because you want to let the user create non 
complaint output.
I'm running in circles here and it's getting tiresome.

> 
> Thanks
> 
> [...]
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>