[FFmpeg-devel] [PATCH 2/4] id3v2: merge TYER/TDAT/TIME to date tag
Aurelien Jacobs
aurel
Wed Nov 3 01:15:23 CET 2010
On Tue, Nov 02, 2010 at 10:16:31PM +0100, Anton Khirnov wrote:
> On Mon, Nov 01, 2010 at 07:54:00AM +0100, Reimar D?ffinger wrote:
> > On Sun, Oct 31, 2010 at 10:38:11PM +0100, Anton Khirnov wrote:
> > > On Sun, Oct 31, 2010 at 06:39:58PM +0100, Reimar D?ffinger wrote:
> > > > > + for (i = 0; i < sizeof(keys); i++) {
> > > > > + AVMetadataTag *t = av_metadata_get(*m, keys[i], NULL, AV_METADATA_MATCH_CASE);
> > > > > + if (!t || strlen(t->value) != 4 || !is_number(t->value))
> > > > > + break;
> > > > > + tags[i] = t;
> > > > > + }
> > > > > +
> > > > > + if (tags[2]) len = sizeof("YYYY-MM-DD HH:MM");
> > > > > + else if (tags[1]) len = sizeof("YYYY-MM-DD");
> > > > > + else if (tags[0]) len = sizeof("YYYY");
> > > > > + else return;
> > > > > +
> > > > > + if (!(date = av_malloc(len)))
> > > > > + return;
> > > > > + snprintf(date, len, "%.4s-%.2s-%.2s %.2s:%.2s",
> > > > > + tags[0] ? tags[0]->value : "", // year
> > > > > + tags[1] ? tags[1]->value + 2 : "", // month
> > > > > + tags[1] ? tags[1]->value : "", // day
> > > > > + tags[2] ? tags[2]->value : "", // hour
> > > > > + tags[2] ? tags[2]->value + 2 : ""); // minute
> > > >
> > > > I suspect this could profit hugely from some restructuring, but at the very
> > > > least I'd suggest adding some variables and doing
> > > > year = tags[0] ? tags[0]->value : "";
> > > > ....
> > > >
> > > i fail to see what that would accomplish other than making it longer
> >
> > Making it a lot easier to understand what it does?
> > In particular, get rid of the comments that are likely to be wrong
> > after the next code change anyway?
> > It might also be possible to combine them with the if (tags[...])
> > checks that are already there anyway.
> ok, done
>
> From bb82dc829c0a89d47f52790a7de277c928fa6488 Mon Sep 17 00:00:00 2001
> From: Anton Khirnov <anton at khirnov.net>
> Date: Sun, 10 Oct 2010 09:21:58 +0200
> Subject: [PATCH] id3v2: merge TYER/TDAT/TIME to date tag
>
> ---
> libavformat/id3v2.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 51 insertions(+), 0 deletions(-)
>
> diff --git a/libavformat/id3v2.c b/libavformat/id3v2.c
> index cc3697a..e92d624 100644
> --- a/libavformat/id3v2.c
> +++ b/libavformat/id3v2.c
> @@ -162,6 +162,56 @@ static void read_ttag(AVFormatContext *s, ByteIOContext *pb, int taglen, const c
> av_metadata_set2(&s->metadata, key, val, 0);
> }
>
> +static int is_number(const char *p)
> +{
> + while (*p) {
> + if (*p < '0' || *p > '9')
> + return 0;
> + p++;
> + }
> + return 1;
> +}
> +
> +static void merge_date(AVMetadata **m)
> +{
> + static const char keys[][5] = {"TYER", "TDAT", "TIME"};
> + AVMetadataTag *tags[] = { NULL , NULL , NULL };
> + char *date, *year, month, day, hour, minute;
> + int i, len = 0;
> +
> + for (i = 0; i < FF_ARRAY_ELEMS(keys); i++) {
> + AVMetadataTag *t = av_metadata_get(*m, keys[i], NULL, AV_METADATA_MATCH_CASE);
> + if (!t || strlen(t->value) != 4 || !is_number(t->value))
> + break;
> + tags[i] = t;
> + }
> +
> + if (tags[0]) {
> + len += sizeof("YYYY") - 1;
> + year = tags[0]->value;
> + } else
> + return;
> + if (tags[1]) {
> + len += sizeof("-MM-DD") - 1;
> + month = tags[1]->value + 2;
> + day = tags[1]->value;
> + } else
> + month = day = "";
> + if (tags[2]) {
> + len += sizeof(" HH:MM") - 1;
> + hour = tags[2]->value;
> + minute = tags[2]->value + 2;
> + } else
> + hour = minute = "";
Computing len seems pretty useless... Just fix its value to 16 and be
done with it. The few allocated bytes that might not be used really
won't hurt.
> + if (!(date = av_malloc(len + 1)))
> + return;
> + snprintf(date, len, "%.4s-%.2s-%.2s %.2s:%.2s", year, month, day, hour, minute);
> + av_metadata_set2(m, "date", date, AV_METADATA_DONT_STRDUP_VAL);
Actually it would be even simpler to just use:
char date[17];
and drop the av_malloc() and the AV_METADATA_DONT_STRDUP_VAL.
Aurel
More information about the ffmpeg-devel
mailing list