After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 339704 - [id3demux] read images from ID3 tags
[id3demux] read images from ID3 tags
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal enhancement
: 0.10.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 340721 341556 341557 343341
Blocks: 339703
 
 
Reported: 2006-04-25 13:21 UTC by Bastien Nocera
Modified: 2006-06-11 20:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
parse APIC frames, take #1 (5.80 KB, patch)
2006-05-12 17:47 UTC, Tim-Philipp Müller
none Details | Review

Description Bastien Nocera 2006-04-25 13:21:27 UTC
This could be used to display images embedded in ID3 tags in some mp3 files.

<hadess> doctau__: is there a gst bug already opened to be able to read thumbnails from id3 tags in gst?
<doctau__> hadess: no idea, but I can tell you exactly where you need to add the code (I was just looking at it)
<doctau__> gst-plugins-good/gst/id3demux/id3v2frame.c:id3demux_id3v2_parse_frame()
Comment 1 Tim-Philipp Müller 2006-04-25 13:41:16 UTC
We should probably add a new tag for that to the core, similar to what was added to the 0.8 branch (GST_TAG_IMAGE or whatever).
Comment 2 Edward Hervey 2006-04-25 15:39:01 UTC
The problem I see with tag, as opposed to creating a new pad, is that you cannot specify the type of data (png ? jpg ? ...?) whereas you can with a pad/stream.
Comment 3 Bastien Nocera 2006-04-25 15:44:22 UTC
As long as the data format is agreed upon at the start, I don't really see a problem either way. A good ol' GdkPixbuf would do fine.
Comment 4 Bastien Nocera 2006-04-25 15:56:58 UTC
Obviously GdkPixbuf is no good, because that would mean relying on GdkPixbuf in the core. Using the same format as GdkPixbuf internally would be good enough.
Comment 5 Tim-Philipp Müller 2006-04-25 15:57:41 UTC
Using a GdkPixbuf as content would mean a link/dependency on libgdkpixbuf for the tag stuff (for the GType when registering the tag) and for the tag reading element (for the decompression), which isn't a good idea IMHO.

I'd just register the new tag to be of GST_TYPE_BUFFER and document it to contain a typefindable compressed image like png or jpeg or gif. Then the application can decide how/when it wants to handle the content, if at all. Typefinding PNG/JPEG/GIF should be easy in any case.

Edward: creating a new pad in this case would mean it gets autoplugged and connected to a videosink, overriding visualisation plugins, wouldn't it? We can easily hack around that of course, but I think a tag would make things much easier for applications, don't you think?
Comment 6 James "Doc" Livingston 2006-04-26 03:08:51 UTC
The other problem with creating a new pad is that it will cause problems applications (such as Rhythmbox, and other music players) that say "it contains a video/image stream, I'm not interested".


(In reply to comment #5)
> I'd just register the new tag to be of GST_TYPE_BUFFER and document it to
> contain a typefindable compressed image like png or jpeg or gif. Then the
> application can decide how/when it wants to handle the content, if at all.
> Typefinding PNG/JPEG/GIF should be easy in any case.

That sounds reasonable to me. Any application that is actually using GDK can use GdkPixbufLoader fairly trivially to convert the data into a GdkPixbuf.
Comment 7 Tim-Philipp Müller 2006-04-27 18:26:09 UTC
Does anyone have any sample files at hand by any chance?

Also, I wonder what to do about the whole 'picture type' situation - does anyone know if that's used in practice? (I imagine '$11: a bright coloured fish' might be ...)
Comment 8 James "Doc" Livingston 2006-04-28 04:19:07 UTC
I don't have any handy, but EasyTag says it will create them and IIRC iTunes embeds cover art in MP3s using this.


(In reply to comment #7)
> Also, I wonder what to do about the whole 'picture type' situation - does
> anyone know if that's used in practice? (I imagine '$11: a bright coloured
> fish' might be ...)

With the possible exception of the "file icon" type, I can't imagine that many applications pay attention to the type. If we did want the type attached to the tag, it would probably need to be turned into a string or something, since other formats may store images with different types.


I'd be happy with just ignoring the picture type, or possibly having an additional IMAGE_PREVIEW tag for the "file icon" type (which could get used for other types that store previews too).
Comment 9 Andy Wingo 2006-05-04 11:36:49 UTC
you could just set the caps on the buffer, no need to typefind...
Comment 10 Tim-Philipp Müller 2006-05-12 17:47:51 UTC
Created attachment 65337 [details] [review]
parse APIC frames, take #1

First tentative patch, requires core + base CVS for now (new releases coming up in the next few days).

Open issues:

  - image URIs: should we create a new tag for that, which would mean we
    couldn't easily pass additional information like picture type etc.; or
    should we just let the app figure out that a mime type of text/uri-list
    means it's an URI?

  - add 'id3-picture-type' number to caps as plain G_TYPE_INT or do we
    need to create and use an enum type for that? (I doubt this picture
    type indicator is used very much)
Comment 11 James "Doc" Livingston 2006-05-13 03:34:39 UTC
(In reply to comment #10)
>   - add 'id3-picture-type' number to caps as plain G_TYPE_INT or do we
>     need to create and use an enum type for that? (I doubt this picture
>     type indicator is used very much)

To me, the most interesting type is the "preview/file icon" type - not particularly for ID3 tags, but for other formats that embed previews. Having a format-neutral way of indicating those (whether something in the caps, or a GST_TAG_IMAGE_PREVIEW tag) would be useful for things like totem-video-thumbnailer.
Comment 12 Tim-Philipp Müller 2006-06-11 20:12:21 UTC
 2006-06-11  Tim-Philipp Müller  <tim at centricular dot net>

       * gst/id3demux/id3v2frames.c: (id3demux_id3v2_parse_frame),
       (scan_encoded_string), (parse_picture_frame):
         Extract images from ID3v2 tags (APIC frames). Fixes #339704.

       * configure.ac:
         Require core >= 0.10.8 (for GST_TAG_IMAGE and
         GST_TAG_PPEVIEW_IMAGE used in the patch above).

Leaving the picture type issue open for now (since I doubt it is used very much and I don't think this is worth blocking another -good release for, since we might need to add stuff to -base, so we'd need another -base release first).

Opened bug #344605 for this.