On Tue, 12 Apr 2011, Luca Barbato wrote:

> On 4/12/11 2:13 PM, Martin Storsjö wrote:
> > As I said in the other thread, if you return from parse_keyframes_index
> > without seeking - like you propose, you need to parse in exactly the same
> > way as what amf_parse_object does.
> 
> An amf object is just a null terminated list of tuples {string, object}

Exactly

> > There's no requirement on exactly what kind of data you can put in AMF,
> > it's a freeform structure as far as I know. The structure currently is
> > something like this:
> > 
> > keyframes: { times: [ 1, 2, 3, 4 ], filepositions: [ 100, 200, 300, 400 ]
> > }
> > 
> > Now what if the keyframes struct (or object in AMR vocabulary) contains an
> > additional key/value, like this:
> > keyframes: { times: [ ... ], foo: 123, filepositions: [ ... ] }
> 
> We will error out since it isn't what we expect, the parser will error out and
> will skip to the next flv packet since the scriptdata packet is wrong for it.

Yes, but "it isn't what we expect" isn't a reason to abort parsing the 
_whole_ onMetaData packet, just because someone has added an extra 
key/value pair in the keyframes mixedarray.

> > The generic parser in amf_parse_object handles this - whenever a generic
> > data value is expected, it parses that data value according to the type
> > identifier (which in most cases just is skipping past the value).
> 
> > parse_keyframes_index only works with this one fixed structure, if it sees
> > something else - anything else than what it expects, it aborts. And then
> > the rest of amr_parse_object can't reliably continue parsing the AMF tree,
> > since parse_keyframes_index possibly left the reader at any undefined
> > position within the keyframes struct.
> 
> So we skip away exactly the same way we do for onMetaData-as-object situations
> and other broken flv.

Skipping a whole packet is one thing if it's a video packet. For flv 
metadata, there is most often only this one single onMetaData packet, and 
if someone adds a single key/value pair somewhere and it breaks parsing 
the whole packet, it sure sounds like a bug in our code to me.

> > So if we want to parse this in a simple one-pass fashion, we need to
> > integrate the content from parse_keyframes_index into the generic
> > recursive parser in amr_parse_object, so that the parser can iterate
> > through whatever AMF values there are. Or call into amf_parse_object for
> > anything that can be found which isn't specifically parsed by
> > parse_keyframes_index.
> 
> That is a slightly more complex option, once I have a sample I'll look into it
> if we want to preserve as much information as possible instead of discarding
> it.
> 
> Summary of flvdec
> 
> - it gets the initial few bytes header that tell basically if you would expect
> audio or video.
> 
> - it might get a scriptdata (so a packet 0x12) that might have the onMetaData
> name. In that case it tries to parse it as mixedarray (since that's what the
> specs know) and then goes to the next packet since it is already known. If the
> parsing has success it will update the metadata accordingly. (NOTE we have the
> same issue discussed in ogg here and a sort of working solution already in
> place)
> 
> - it might get a video packet, if it belongs to a known stream then it is just
> demuxed, otherwise a new stream is added and then demuxed.
> 
> - it might get an audio packet, same behaviour as for the video.
> 
> - aac, h264 and other extradata rich codecs have per packet extradata in case
> the one present in onMetaData is stale or missing.
> 
> In my data branch I'm adding the ability to preserve the other scriptdata
> packets and possibly embed other misc data there once I have a working
> testcase and librtmp fixes in place might be possible to have soft subtitles
> in rtmp/flv.
> 
> I hope I got the code right.

Yes, that sounds about right, while quite unrealted to the internal 
parsing of onMetaData.

// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to