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

[Michael Niedermayer]

Hi

On Mon, Dec 11, 2006 at 03:57:14AM -0800, Allan Hsu wrote:
> 
> On Dec 11, 2006, at 12:49 AM, Allan Hsu wrote:
> 
> [...]
> >>
> >>IMHO these should be set directly in AVCodecContext without the  
> >>intermediate
> >>FLVDemuxContext layer
> >
> >Same as above. New patch attached that writes metadata straight  
> >into the streams.
> 
> After more testing I noticed that files with samplerate but otherwise  
> incomplete audio metadata would not decode properly. Here's a fixed  
> version of the patch.


[...]

> +        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


[...]
> +            else if(!strcmp(key, "audiocodecid")) {
> +                switch((int)num_val << FLV_AUDIO_CODECID_OFFSET) {
> +                    //no distinction between S16 and S8 PCM codec flags
> +                    case FLV_CODECID_PCM_BE:
> +                        acodec->codec_id = acodec->bits_per_sample == 8 ? CODEC_ID_PCM_S8 : CODEC_ID_PCM_S16BE; break;
> +                    case FLV_CODECID_PCM_LE:
> +                        acodec->codec_id = acodec->bits_per_sample == 8 ? CODEC_ID_PCM_S8 : CODEC_ID_PCM_S16LE; break;
> +                    case FLV_CODECID_ADPCM: acodec->codec_id = CODEC_ID_ADPCM_SWF;                              break;
> +                    case FLV_CODECID_MP3  : acodec->codec_id = CODEC_ID_MP3      ; astream->need_parsing = 1  ; break;
> +                    case FLV_CODECID_NELLYMOSER_8HZ_MONO:
> +                        acodec->sample_rate = 8000; //in case metadata does not otherwise declare samplerate
> +                    case FLV_CODECID_NELLYMOSER:
> +                    default:
> +                        acodec->codec_tag = num_val;
> +                }

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


> +            }
> +            else if(!strcmp(key, "videocodecid")) {
> +                switch((int)num_val) {
> +                    case FLV_CODECID_H263  : vcodec->codec_id = CODEC_ID_FLV1   ; break;
> +                    case FLV_CODECID_SCREEN: vcodec->codec_id = CODEC_ID_FLASHSV; break;
> +                    case FLV_CODECID_VP6   : vcodec->codec_id = CODEC_ID_VP6F   ; break;
> +                    default:
> +                        vcodec->codec_tag = num_val;
> +                }
> +            }
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int flv_read_metabody(AVFormatContext *s, unsigned int next_pos) {
> +    AMFDataType type;
> +    AVStream *stream, *astream, *vstream;
> +    ByteIOContext *ioc;
> +    int i, keylen;
> +    unsigned int maxindex;
> +    char buffer[256];
> +
> +    astream = NULL;
> +    vstream = NULL;
> +    keylen = 0;
> +    ioc = &s->pb;
> +
> +    //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?


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin