Patch: ASF picture parser
setosha at gmail.com
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 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...
Size: 6776 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/taglib-devel/attachments/20100905/2e004c74/attachment.zip
More information about the taglib-devel