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

[Aurelien Jacobs]

On Fri, 8 Dec 2006 00:41:32 -0800
allan at counterpop.net (Allan Hsu) wrote:

> This is my second try at the FLV decoder metadata patch. I've tried to
> adhere to the submission guidelines and Michael's suggestions. Please
> let me know if this patch is acceptable or if I still need to do
> anything before it can be merged in SVN.
> 
> [...]
> 
> +typedef struct {
> +    int hasVideo;
> +    int hasAudio;
> [...]
> +    int isStereo;

Please avoid camelCase style in var name.

> +    switch(objectType) {
> +        case      AMF_DATA_TYPE_NUMBER:
> +            url_fskip(ioc, 8); break; //double precision float
> +        case        AMF_DATA_TYPE_BOOL:
> +            url_fskip(ioc, 1); break; //byte
> +        case      AMF_DATA_TYPE_STRING:

Here your indentation style looks odd. It would be better this way:

+    switch(objectType) {
+        case AMF_DATA_TYPE_NUMBER:
+            url_fskip(ioc, 8); break; //double precision float
+        case AMF_DATA_TYPE_BOOL:
+            url_fskip(ioc, 1); break; //byte
+        case AMF_DATA_TYPE_STRING:

(same applies to other parts of your patch)

> +        if(       strcmp(buffer, "duration")        == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) s->duration           = dbl * AV_TIME_BASE;
> +        } else if(strcmp(buffer, "width")           == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->width        = dbl;
> +        } else if(strcmp(buffer, "height")          == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->height       = dbl;
> +        } else if(strcmp(buffer, "audiocodecid")    == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->audiocodecid = dbl;
> +        } else if(strcmp(buffer, "videocodecid")    == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->videocodecid = dbl;
> +        } else if(strcmp(buffer, "stereo")          == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_BOOL  , &bool) == 0) context->isStereo     = bool;
> +        } else if(strcmp(buffer, "audiosamplerate") == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->samplerate   = dbl;
> +        } else if(strcmp(buffer, "audiosamplesize") == 0) {
> +            if(amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl ) == 0) context->samplesize   = dbl;
> +        } else if(amf_skip_object(ioc, NULL) < 0) {
> +            goto bail;
> +        }

Maybe this could be simplified with something like this
(note that it is untested):

+        if (!strcmp(buffer, "stereo")) {
+            if (!amf_get_object(ioc, AMF_DATA_TYPE_BOOL, &bool))
+                context->isStereo = bool;
+        } else if (!amf_get_object(ioc, AMF_DATA_TYPE_NUMBER, &dbl) {
+            if(!strcmp(buffer, "duration"))      s->duration = AV_TIME_BASE * dbl;
+            else if(!strcmp(buffer, "width"))         context->width        = dbl;
+            else if(!strcmp(buffer, "height"))        context->height       = dbl;
+            else if(!strcmp(buffer, "audiocodecid"))  context->audiocodecid = dbl;
+            else if(!strcmp(buffer, "videocodecid"))  context->videocodecid = dbl;
+            else if(!strcmp(buffer, "audiosamplerate")) context->samplerate = dbl;
+            else if(!strcmp(buffer, "audiosamplesize")) context->samplesize = dbl;
+        }

Aurel