[Bug 566605] Support the new libav metadata API

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=566605

--- Comment #5 from Jan Schmidt <thaytan at noraisin.net> ---
(In reply to Nicolas Dufresne (stormer) from comment #4)
> Review of attachment 308453 [details] [review]:

Thanks for the review :)

> ::: ext/libav/gstavutils.c
> @@ +494,3 @@
> +
> +static gchar *
> +my_safe_copy (gchar * input)
> 
> Hmm, using my_ is uncommon in gstreamer ;-P

Yeah - I just moved the existing function to the new file without renaming.
Turns out it should probably all just stay in gstavdemux.c

> @@ +524,3 @@
> +    g_hash_table_insert (tmp, (void *) "comment", (void *) GST_TAG_COMMENT);
> +    g_hash_table_insert (tmp, (void *) "composer", (void *)
> GST_TAG_COMPOSER);
> +    g_hash_table_insert (tmp, (void *) "copyright", (void *)
> GST_TAG_COPYRIGHT);
> 
> There is usually no reason in C to cast to void *.

It's a const thing - g_hash_table takes non-const pointers.


> @@ +526,3 @@
> +    g_hash_table_insert (tmp, (void *) "copyright", (void *)
> GST_TAG_COPYRIGHT);
> +    //g_hash_table_insert (tmp, "creation_time", GST_TAG_DATE_TIME); /*
> Need to convert ISO 8601 to GstDateTime */
> +    //g_hash_table_insert (tmp, "date", GST_TAG_DATE); /* ditto */
> 
> C++ style comments. And why is there something commented out ?

I cut and pasted the list of all standard tag names from the ffmpeg header, but
ran out of time to implement them all. I wanted to put the patch up so it
doesn't get lost. I'll add a FIXME.

> 
> ::: ext/libav/gstavutils.h
> @@ +96,3 @@
>  
> +GstTagList *
> +gst_ffmpeg_metadata_to_tag_list (AVDictionary * metadata);
> 
> Should on same line, but I see you are following already in place style.

Ja.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.