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

[Michael Niedermayer]

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

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- 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/20130924/13b11ad8/attachment.asc>