[FFmpeg-devel] [PATCH] FLV metainjection(similartoFLVTOOLS2butan addon patch for FFMPEG)

Michael Niedermayer michaelni
Tue Aug 14 03:54:51 CEST 2007


Hi

On Tue, Aug 14, 2007 at 12:54:49AM +0000, andrew wrote:
> > On Mon, Aug 13, 2007 at 10:56:25AM +0200, Reimar D?ffinger wrote:
> > > On Mon, Aug 13, 2007 at 09:09:28AM +0100, M?ns Rullg?rd wrote:
> > > > Luca Barbato <lu_zero at gentoo.org> writes:
> > > > > M?ns Rullg?rd wrote:
> > > > >> andrew <andrew at butkus.co.uk> writes:
> > > > >> 
> > > > >> [top-post with long lines and senselessly long sig]
> > > > >> 
> > > > >> Please, don't top-post, wrap your lines, and don't use 20-line
> > > > >> signatures.
> > > > >
> > > > > And attach patches as plain text.
> > > > 
> > > > text/x-patch is preferred, though text/plain is tolerable.
> > > 
> > > Actually I still prefer text/plain until I finally find a way to make
> > > mutt quote text/x-patch inline (actually text/* IMO should be treated
> > > like that, I once did a patch to do that but it was a mess, hacking
> > > quite a lot of hardcoded string comparisons).
> > 
> > .. works here ..
> > 
> > Diego
> 
> I updated some more tags for flv meta injection (for instance 
> 'metadatacreator:')
[...]
> @@ -3675,6 +3682,7 @@
>      { "vtag", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_video_tag}, "force video tag/fourcc", "fourcc/tag" },
>      { "newvideo", OPT_VIDEO, {(void*)opt_new_video_stream}, "add a new video stream to the current output stream" },
>      { "qphist", OPT_BOOL | OPT_EXPERT | OPT_VIDEO, { (void *)&qp_hist }, "show QP histogram" },
> +    { "flvmeta_inject", 0, {(void*)opt_flvmeta_inject}, "add flv meta keyframe data" },

is there a reason why a user would want to omit the index aka 
"flv meta keyframe data"?
if not theres no need for this parameter and a few other parts of this
patch


[...]
> @@ -202,6 +208,57 @@
>      flv->duration_offset= url_ftell(pb);
>      put_amf_double(pb, 0); // delayed write
>  
> +    // enable meta writing,    

trailing whitespace
try a
grep ' $' myfile.c
to find them all
and remove them, traling whitespace is forbidden in ffmpeg svn


[...]
> +        put_amf_string(pb, "metadatacreator");
> +        put_byte(pb, 2);
> +        put_amf_string(pb, CREATOR_TAG);

what you want to use here is LIBAVFORMAT_IDENT


> +
> +        put_amf_string(pb, "lastkeyframetimestamp");    
> +        flv->lastkeyframe_offset= url_ftell(pb);
> +        put_amf_double(pb, 0);
> +
> +        put_amf_string(pb, "keyframes");
> +        put_byte(pb, AMF_DATA_TYPE_OBJECT); // Imitates break to child node
> +        put_amf_string(pb, "times");
> +        put_byte(pb, AMF_DATA_TYPE_ARRAY); // Imitates child node value
> +
> +        for (i = 0; i <= 2; i++)
> +            put_byte(pb, 0);
> +        put_byte(pb, NUM_OF_ENTRIES); //number of time entries
> +
> +        //time entries (place holders)
> +        flv->time_offset= url_ftell(pb);
> +        for (i = 0; i <= NUM_OF_ENTRIES-1; i++)
> +            put_amf_double(pb, 0);
> +
> +        put_amf_string(pb, "filepositions");
> +        put_byte(pb, AMF_DATA_TYPE_ARRAY);
> +
> +        for (i = 0; i <= 2; i++)
> +            put_byte(pb, 0);
> +        put_byte(pb, NUM_OF_ENTRIES);
> +
> +        //time entries (place holders)
> +        flv->fposition_offset= url_ftell(pb);
> +        for (i = 0; i <= NUM_OF_ENTRIES-1; i++)
> +            put_amf_double(pb, 0);

cant the index be stored at the end of the file? that would simplify this
if it cannot be stored the end then another solution must be found, reserving
(wasting) a fixed amount at the begin is not ok


[...]
> @@ -265,6 +322,36 @@
>      url_fseek(pb, flv->filesize_offset, SEEK_SET);
>      put_amf_double(pb, file_size);
>  
> +    if(s->flvmeta_inject){
> +        int duration = flv->duration / (double)1000;

the cast does not do anything usefull here, please remove it


> +
> +        url_fseek(pb, flv->lastkeyframe_offset, SEEK_SET);
> +        put_amf_double(pb, duration);
> +
> +        int i;
> +        int flvduration = NUM_OF_ENTRIES-1;

mixing declarations and statements violates the ISO C90, please reorder
the lines ...


> +        
> +        if (duration < flvduration)
> +            flvduration = duration;
> +
> +        int steptime = duration / flvduration;  
> +
> +        url_fseek(pb, flv->time_offset, SEEK_SET);
> +        for (i = 0; i <= flvduration; i++)
> +            put_amf_double(pb, i*steptime);
> +
> +        int stepfpos = file_size / flvduration;
> +
> +        url_fseek(pb, flv->fposition_offset, SEEK_SET);
> +        for (i = 0; i <= flvduration; i++)
> +            put_amf_double(pb, i*stepfpos);

no you must store the proper timestamps and positions, i*any_constant
is wrong


[...]
> @@ -94,17 +98,18 @@
>  };
>  
>  typedef enum {
> -    AMF_DATA_TYPE_NUMBER      = 0x00,
> -    AMF_DATA_TYPE_BOOL        = 0x01,
> -    AMF_DATA_TYPE_STRING      = 0x02,
> -    AMF_DATA_TYPE_OBJECT      = 0x03,
> -    AMF_DATA_TYPE_NULL        = 0x05,
> -    AMF_DATA_TYPE_UNDEFINED   = 0x06,
> -    AMF_DATA_TYPE_REFERENCE   = 0x07,
> -    AMF_DATA_TYPE_MIXEDARRAY  = 0x08,
> -    AMF_DATA_TYPE_ARRAY       = 0x0a,
> -    AMF_DATA_TYPE_DATE        = 0x0b,
> -    AMF_DATA_TYPE_UNSUPPORTED = 0x0d,
> +    AMF_DATA_TYPE_NUMBER         = 0x00,
> +    AMF_DATA_TYPE_BOOL           = 0x01,
> +    AMF_DATA_TYPE_STRING         = 0x02,
> +    AMF_DATA_TYPE_OBJECT         = 0x03,
> +    AMF_DATA_TYPE_NULL           = 0x05,
> +    AMF_DATA_TYPE_UNDEFINED      = 0x06,
> +    AMF_DATA_TYPE_REFERENCE      = 0x07,
> +    AMF_DATA_TYPE_MIXEDARRAY     = 0x08,
> +    AMF_DATA_TYPE_ARRAY          = 0x0a,
> +    AMF_DATA_TYPE_DATE           = 0x0b,
> +    AMF_DATA_TYPE_UNSUPPORTED    = 0x0d,
> +    AMF_DATA_TYPE_PARENTNODE     = 0x09,

cosmetics and its not properly sorted


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

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070814/fcc0032d/attachment.pgp>



More information about the ffmpeg-devel mailing list