[7/8] movenc: Factorize a function for finding a metadata entry and the associated language

Message ID 1463569341-73691-7-git-send-email-martin@martin.st
State Committed
Headers show

Commit Message

Martin Storsjö May 18, 2016, 11:02 a.m.
---
 libavformat/movenc.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

Comments

Diego Biurrun May 18, 2016, 2:32 p.m. | #1
On Wed, May 18, 2016 at 02:02:20PM +0300, Martin Storsjö wrote:
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -2004,16 +2004,17 @@ static int mov_write_string_tag(AVIOContext *pb, const char *name,
> +static AVDictionaryEntry *get_metadata_lang(AVFormatContext *s,
> +                                            const char *tag, int *lang)
>  {
>  
> +    *lang = 0;
> +
> @@ -2021,10 +2022,21 @@ static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
> +
> +static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
> +                                     const char *name, const char *tag,
> +                                     int long_style)
> +{
> +    int lang = 0;
> +    AVDictionaryEntry *t = get_metadata_lang(s, tag, &lang);
> +    if (!t)
> +        return 0;
>      return mov_write_string_tag(pb, name, t->value, lang, long_style);
>  }

That you (redundantly) set lang to 0 is a bit confusing.  Is there a
reason?

Diego
Martin Storsjö May 18, 2016, 7:14 p.m. | #2
On Wed, 18 May 2016, Diego Biurrun wrote:

> On Wed, May 18, 2016 at 02:02:20PM +0300, Martin Storsjö wrote:
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -2004,16 +2004,17 @@ static int mov_write_string_tag(AVIOContext *pb, const char *name,
>> +static AVDictionaryEntry *get_metadata_lang(AVFormatContext *s,
>> +                                            const char *tag, int *lang)
>>  {
>> 
>> +    *lang = 0;
>> +
>> @@ -2021,10 +2022,21 @@ static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
>> +
>> +static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
>> +                                     const char *name, const char *tag,
>> +                                     int long_style)
>> +{
>> +    int lang = 0;
>> +    AVDictionaryEntry *t = get_metadata_lang(s, tag, &lang);
>> +    if (!t)
>> +        return 0;
>>      return mov_write_string_tag(pb, name, t->value, lang, long_style);
>>  }
>
> That you (redundantly) set lang to 0 is a bit confusing.  Is there a
> reason?

Not really, I guess I can leave it out from the variable declaration here. 
Ok with that changed?

// Martin
Diego Biurrun May 19, 2016, 7:38 a.m. | #3
On Wed, May 18, 2016 at 10:14:12PM +0300, Martin Storsjö wrote:
> On Wed, 18 May 2016, Diego Biurrun wrote:
> 
> > On Wed, May 18, 2016 at 02:02:20PM +0300, Martin Storsjö wrote:
> >> --- a/libavformat/movenc.c
> >> +++ b/libavformat/movenc.c
> >> @@ -2004,16 +2004,17 @@ static int mov_write_string_tag(AVIOContext *pb, const char *name,
> >> +static AVDictionaryEntry *get_metadata_lang(AVFormatContext *s,
> >> +                                            const char *tag, int *lang)
> >>  {
> >> 
> >> +    *lang = 0;
> >> +
> >> @@ -2021,10 +2022,21 @@ static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
> >> +
> >> +static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
> >> +                                     const char *name, const char *tag,
> >> +                                     int long_style)
> >> +{
> >> +    int lang = 0;
> >> +    AVDictionaryEntry *t = get_metadata_lang(s, tag, &lang);
> >> +    if (!t)
> >> +        return 0;
> >>      return mov_write_string_tag(pb, name, t->value, lang, long_style);
> >>  }
> >
> > That you (redundantly) set lang to 0 is a bit confusing.  Is there a
> > reason?
> 
> Not really, I guess I can leave it out from the variable declaration here. 
> Ok with that changed?

Yes.

Diego

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 9b36ca9..628f7aa 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2004,16 +2004,17 @@  static int mov_write_string_tag(AVIOContext *pb, const char *name,
     return size;
 }
 
-static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
-                                     const char *name, const char *tag,
-                                     int long_style)
+static AVDictionaryEntry *get_metadata_lang(AVFormatContext *s,
+                                            const char *tag, int *lang)
 {
-    int l, lang = 0, len, len2;
+    int l, len, len2;
     AVDictionaryEntry *t, *t2 = NULL;
     char tag2[16];
 
+    *lang = 0;
+
     if (!(t = av_dict_get(s->metadata, tag, NULL, 0)))
-        return 0;
+        return NULL;
 
     len = strlen(t->key);
     snprintf(tag2, sizeof(tag2), "%s-", tag);
@@ -2021,10 +2022,21 @@  static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
         len2 = strlen(t2->key);
         if (len2 == len + 4 && !strcmp(t->value, t2->value)
             && (l = ff_mov_iso639_to_lang(&t2->key[len2 - 3], 1)) >= 0) {
-            lang = l;
-            break;
+            *lang = l;
+            return t;
         }
     }
+    return t;
+}
+
+static int mov_write_string_metadata(AVFormatContext *s, AVIOContext *pb,
+                                     const char *name, const char *tag,
+                                     int long_style)
+{
+    int lang = 0;
+    AVDictionaryEntry *t = get_metadata_lang(s, tag, &lang);
+    if (!t)
+        return 0;
     return mov_write_string_tag(pb, name, t->value, lang, long_style);
 }