[FFmpeg-devel] [PATCH] Store Major brand, Minor version and compatible brands of a mov file using the metadata API

haim alon haim.alter
Wed Sep 9 12:42:17 CEST 2009


On Wed, Sep 9, 2009 at 11:50 AM, Diego Biurrun <diego at biurrun.de> wrote:

> On Wed, Sep 09, 2009 at 10:03:04AM +0300, haim alon wrote:
> >
> > On Tue, Sep 8, 2009 at 8:34 PM, Baptiste Coudurier wrote:
> >
> > > On 09/08/2009 04:55 AM, haim alon wrote:
> > >
> > > Please make the patch coding style and variable naming scheme matching
> the
> > > one used in the same file.
>
> Please heed Baptiste's request.  Don't just mechanically make exactly
> the changes you are told to make, look for yourself if there are
> similar changes that you need to make in other places.
>
> > > compat_brand, minor_ver, etc...
> > >
> > Done
>
> No.
>
> > >> --- libavformat/mov.c   (revision 19787)
> > >> +++ libavformat/mov.c   (working copy)
> > >> @@ -485,15 +485,51 @@
> > >  +    av_metadata_set(&c->fc->metadata, "MajorBrand", majorBrandStr);
> > >
> > > "major_brand"
> > >
> > Done
>
> No.
>
> > >  +    minorVer = get_be32(pb); /*minor version*/
> > >> +    snprintf(minorVerStr, sizeof(minorVerStr), "%d", minorVer);
> > >> +    av_metadata_set(&c->fc->metadata, "MinorVersion", minorVerStr);
> > >
> > > "minor_version"
> > >
> > Done
>
> No.
>
> > >> +        if (nextCompBrandPtr[0]&&  nextCompBrandPtr[1]&&
> nextCompBrandPtr[2]&&  nextCompBrandPtr[3]) // check that char is legal -
> not NULL
> > >> +        {
> > >> +            memcpy(currCompBrandPtr,&nextCompBrand, 4);
> > >> +            currCompBrandPtr += 4;
> > >> +        }
> > >> +        else
> > >> +        {
> > >> +            av_log(c->fc, AV_LOG_WARNING, "compatible brand name
> contains illegal character - skipped.\n");
> > >> +            numCompBrand -= 1; /* reduce counter since brand is
> skipped */
> > >> +        }
> > >
> > > Brace placements.
> > >
> > Done
>
> No.
>
> > >  +    }
> > >> +    *currCompBrandPtr = '\0';
> > >> +    av_metadata_set(&c->fc->metadata, "CompatibleBrands",
> compBrandsStr);
> > >
> > > compatible_brands
> > >
> > Done
>
> No.
>
> > Attached an updated patch.
>
> Try again please, this time follow the style in the file, don't force
> your personal style upon it.
>
> Diego
>

It's my first patch so you can figure out why there are so much code style
difference.
As far as I can see, the remaining style issue is just changing the variable
names.
Thanks for your kindness feedback.

Haim.

_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>



More information about the ffmpeg-devel mailing list