[FFmpeg-devel] [PATCH 1/3] avcodec/tiff_common: add ff_tadd_bytes_metadata()

[Michael Niedermayer]

On Tue, Sep 24, 2013 at 10:22:46PM +0200, Michael Niedermayer wrote:
> On Tue, Sep 24, 2013 at 09:28:56PM +0200, Clément Bœsch wrote:
> > On Tue, Sep 24, 2013 at 09:01:08PM +0200, Michael Niedermayer wrote:
> > > The le argument is passed so the function has the same prototype as the
> > > other similar functions. It is otherwise unused
> > > 
> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > > ---
> > >  libavcodec/tiff_common.c |   31 +++++++++++++++++++++++++++++++
> > >  libavcodec/tiff_common.h |    6 ++++++
> > >  2 files changed, 37 insertions(+)
> > > 
> > > diff --git a/libavcodec/tiff_common.c b/libavcodec/tiff_common.c
> > > index b7bd587..1300935 100644
> > > --- a/libavcodec/tiff_common.c
> > > +++ b/libavcodec/tiff_common.c
> > > @@ -207,6 +207,37 @@ int ff_tadd_shorts_metadata(int count, const char *name, const char *sep,
> > >  }
> > >  
> > >  
> > > +int ff_tadd_bytes_metadata(int count, const char *name, const char *sep,
> > > +                           GetByteContext *gb, int le, AVDictionary **metadata)
> > > +{
> > > +    AVBPrint bp;
> > > +    char *ap;
> > > +    int i;
> > > +
> > > +    if (count >= INT_MAX / sizeof(int8_t) || count <= 0)
> > > +        return AVERROR_INVALIDDATA;
> > > +    if (bytestream2_get_bytes_left(gb) < count * sizeof(int8_t))
> > > +        return AVERROR_INVALIDDATA;
> > > +    if (!sep) sep = ", ";
> > > +
> > > +    av_bprint_init(&bp, 10 * count, AV_BPRINT_SIZE_AUTOMATIC);
> > > +
> > > +    for (i = 0; i < count; i++) {
> > > +        av_bprintf(&bp, "%s%i", (i ? sep : ""), bytestream2_get_byte(gb));
> > > +    }
> > > +
> > > +    if ((i = av_bprint_finalize(&bp, &ap))) {
> > > +        return i;
> > > +    }
> > > +    if (!ap) {
> > > +        return AVERROR(ENOMEM);
> > > +    }
> > > +
> > > +    av_dict_set(metadata, name, ap, AV_DICT_DONT_STRDUP_VAL);
> > > +
> > > +    return 0;
> > > +}
> > > +
> > 
> > Why isn't this optional? Won't this make it unusable/a real pain for apps
> > to deal with this?
> > 
> > Instead, I would suggest to add some escaping the print_string() callback
> > of the default writer (maybe optionnal), which would replace any non
> > printable char with a "\\x%02X" value.
> 
> exif supports the following data types:
> 1 = BYTE An 8-bit unsigned integer.,
> 2 = ASCII An 8-bit byte containing one 7-bit ASCII code. The final byte is terminated with NULL.,
> 3 = SHORT A 16-bit (2-byte) unsigned integer,
> 4 = LONG A 32-bit (4-byte) unsigned integer,
> 5 = RATIONAL
> Two LONGs. The first LONG is the numerator and the second LONG expresses the
> denominator.,
> 7 = UNDEFINED An 8-bit byte that can take any value depending on the field definition,
> 9 = SLONG A 32-bit (4-byte) signed integer (2's complement notation),
> 10 = SRATIONAL
> Two SLONGs. The first SLONG is the numerator and the second SLONG is the
> 
> Currently BYTE and undefined are treated as strings
> 
> this patchset treats bytes and undefined along the same lines as
> short and double.
> which of these do you suggest to be interpreted as strings ?
> and which kind of strings ? ASCII ? UTF8 ?
> 
> note, for correct printing the tag name has to be used i think, and
> from that the type would then be known and could be optimally
> presented to the user like version 2.2.1 instead of "221"
> i dont volunteer to imlement that though ATM i just wanted to make the
> selftests on AIX and openbsd pass at the same time ...

patch approved by ubitux on IRC

applied

thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130929/73e927a9/attachment.asc>