Patch: ASF picture parser

Anton Sergunov 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 p;
p->setDesctiption("ddd");
Picture p2 = p;
p2->setDescription("d2d2d2");
p->description();

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.

by http://msdn.microsoft.com/en-us/library/dd743745(v=VS.85).aspx
can be five types of binary attribute by atribute name:
WM_PICTURE "WM/Picture"
WM_STREAM_TYPE_INFO "WM/StreamTypeInfo"
WM_SYNCHRONISED_LYRICS "WM/Lyrics_Synchronised"
WM_USER_TEXT "WM/Text"
WM_USER_WEB_URL "WM/UserWebURL"

I still don't have idea how to refuse to set picture to anything but
WM\Picture.
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.

removed

In general, what do you think about adding minimal picture
functionality to TagLib::Tag? All modern programs shows cover art in
interface.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: asfpics.zip
Type: application/zip
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 mailing list