[Ffmpeg-devel] [PATCH] FLV decoder metadata reading

[Michael Niedermayer]

Hi

On Mon, Dec 11, 2006 at 03:39:06PM -0800, Allan Hsu wrote:
> 
> On Dec 11, 2006, at 5:13 AM, Michael Niedermayer wrote:
> 
> [...]
> >>+        case AMF_DATA_TYPE_MIXEDARRAY:
> >>+            url_fskip(ioc, 4); //skip 32-bit max array index
> >>+            while(url_ftell(ioc) < max_pos - 2 && amf_get_string 
> >>(ioc, str_val, sizeof(str_val)) > 0) {
> >>+                if(amf_parse_object(s, NULL, NULL, NULL, max_pos)  
> >>< 0)
> >>+                    return -1;
> >>+            }
> >>+                if(get_byte(ioc) != AMF_END_OF_OBJECT)
> >>+                    return -1;
> >>+                break;
> >
> >funny indention ...
> >
> >
> >[...]
> >>+            else if(!strcmp(key, "width")  && vcodec && num_val  
> >>>= 0) vcodec->width  = num_val;
> >>+            else if(!strcmp(key, "height") && vcodec && num_val  
> >>>= 0) vcodec->height = num_val;
> >
> >0 width/height isnt valid
> [...]
> >looking at the code in flv_read_packet():
> >            switch(flags & FLV_AUDIO_CODECID_MASK) {
> >            case FLV_CODECID_PCM_BE: if (flags &  
> >FLV_AUDIO_SAMPLESIZE_MASK) st->codec->codec_id = CODEC_ID_PCM_S16BE;
> >                    else st->codec->codec_id = CODEC_ID_PCM_S8; break;
> >            case FLV_CODECID_ADPCM : st->codec->codec_id =  
> >CODEC_ID_ADPCM_SWF;                       break;
> >            case FLV_CODECID_MP3   : st->codec->codec_id =  
> >CODEC_ID_MP3      ; st->need_parsing = 1; break;
> >            // this is not listed at FLV but at SWF, strange...
> >            case FLV_CODECID_PCM_LE: if (flags &  
> >FLV_AUDIO_SAMPLESIZE_MASK) st->codec->codec_id = CODEC_ID_PCM_S16LE;
> >                    else st->codec->codec_id = CODEC_ID_PCM_S8; break;
> >            default:
> >                    av_log(s, AV_LOG_INFO, "Unsupported audio codec  
> >(%x)\n", flags >> 4);
> >                st->codec->codec_tag= (flags >> 4);
> >            }
> >
> >this and the case for video too can be simplified by moving the  
> >code in
> >a common function
> [...]
> >>+    //first object needs to be "onMetaData" string
> >>+    type = get_byte(ioc);
> >>+    if(type != AMF_DATA_TYPE_STRING || amf_get_string(ioc,  
> >>buffer, sizeof(buffer)) < 0 || strcmp(buffer, "onMetaData") != 0)
> >>+        return -1;
> >>+
> >>+    //second object needs to be a mixedarray
> >>+    type = get_byte(ioc);
> >>+    if(type != AMF_DATA_TYPE_MIXEDARRAY)
> >>+        return -1;
> >
> >cant a single amf_parse_object() call be used here instead of the  
> >special
> >case for AMF_DATA_TYPE_MIXEDARRAY with a loop with amf_parse_object 
> >() calls?
> [...]
> 
> All fixed/changed in the attached version of the patch.

patch looks ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes