Patch: ASF picture parser

Anton Sergunov setosha at
Sun Sep 5 06:32:18 CEST 2010

>  - ASF::Picture should be in a separate file. It should use no private
> member variables or inline methods to make it possible to change it
> without breaking binary compatibility. It would be nice if the private
> d pointer was refcounted, like in MP4::CoverArt.

done. But I'm not shure about code like
Picture p;
Picture p2 = p;

Is this expected behavior?

>  - I'm not sure about the integration with ASF::Attribute. While it's
> useful, it doesn't follow the physical structure of ASF metadata
> attributes. I expect there are more binary attributes than WM/Picture
> and I'm not sure if they all deserve to have special parsers there.
> But if it stays there, I think it should refuse to use the Picture
> class for anything else than "WM/Picture".

Now Picture parses from byte vector.
It still clears data vector in parser if picture loaded. I think
picture can have realy big size and I don't want to store it twice.

If let render() to be public Picture can be removed from Attribute.

can be five types of binary attribute by atribute name:

I still don't have idea how to refuse to set picture to anything but
ASF::Attribute don't know own name
ASF::Tag::setAttribute and ASF::Tag::addAttribute don't have return code.

>  - readNullTerminatedString is probably too specific to be in
> ASF::File, it's not used anywhere but the WM/Picture parsing code. You
> can also use ByteVector::find for the main loop in the function, see
> MPEG::ID3v2::Frame::readStringField for example.


In general, what do you think about adding minimal picture
functionality to TagLib::Tag? All modern programs shows cover art in
-------------- next part --------------
A non-text attachment was scrubbed...
Type: application/zip
Size: 6776 bytes
Desc: not available
Url : 

More information about the taglib-devel mailing list