[Ffmpeg-devel] [PATCH] Read AVI tags

[Michael Niedermayer]

Hi

On Wed, Aug 16, 2006 at 07:59:24PM -0400, David Conrad wrote:
> Hi,
> 
> On Aug 16, 2006, at 4:40 AM, Michael Niedermayer wrote:
> >Hi
> >
> >On Tue, Aug 15, 2006 at 03:25:07PM -0400, David Conrad wrote:
> >>On Aug 15, 2006, at 4:00 AM, Benjamin Larsson wrote:
> >>
> >>>Hi,
> >>>
> >>>David Conrad skrev:
> >>>>On Aug 15, 2006, at 1:39 AM, David Conrad wrote:
> >>>>
> >>>>>Hi,
> >>>>>The following patch adds some read support for metadata tags in
> >>>>>AVI files. One note: the artist tag is put in the author field
> >>>>>since there is no AVI tag for author, but this seems to be the
> >>>>>same behavior as the mp3 parser's id3 tag reader.
> >>>>
> >>>>My apologies, please ignore that last patch. The correct one  
> >>>>follows.
> >>>>
> >>>>-David
> >>>>
> >>>
> >>>can you implement writing also ? You can look here for a patch
> >>>noone cleaned up enough to commit. http://thread.gmane.org/
> >>>gmane.comp.video.ffmpeg.devel/28596/focus=28596
> >>
> >>Okay, I cleaned up that patch for writing, and I think I addressed
> >>all of your and Micheal's concerns with the original. One thing,
> >>though, is that the hash for the two test AVI files for ffserver
> >>changed for me; the original patch doesn't indicate this.
> >>
> >>-David
> >
> >>Index: libavformat/avienc.c
> >>===================================================================
> >>--- libavformat/avienc.c	(revision 5999)
> >>+++ libavformat/avienc.c	(working copy)
> >>@@ -89,6 +89,19 @@
> >>     return tag;
> >> }
> >>
> >>+static void avi_write_info_tag(ByteIOContext *pb, const char  
> >>*tag, const char *contents)
> >>+{
> >>+    int len = strlen(contents);
> >>+    if (len > 0) {
> >>+        len++;
> >>+        put_tag(pb, tag);
> >>+        put_le32(pb, len);
> >>+        put_buffer(pb, contents, len);
> >>+        if (len & 1)
> >>+            put_byte(pb, 0);
> >>+    }
> >>+}
> >
> >this could use put_strz()
> >
> >
> >[...]
> >>+    avi_write_info_tag(pb, "ISFT", "libavformat " AV_STRINGIFY 
> >>(LIBAVFORMAT_VERSION));
> >
> >this should be under CODEC_FLAG_BITEXACT check so regression tests  
> >dont break
> >if the version changes
> >
> >
> >>+    end_tag(pb, list2);
> >>+    list2 = start_tag(pb, "JUNK");
> >>+    for (i = 0; i < 1016; i += 4)
> >>+        put_le32(pb, 0);
> >>+    end_tag(pb, list2);
> >
> >what is that good for?
> 
> It provides expansion space for editing tags without the need to  
> rewrite the entire file. Most muxers give between one and two  
> kilobytes for this.
> 
> How does this version look?

looks mostly ok, somehow i dont like wasting a kb but well, i guess
its not worth to make it optional ... so iam fine with the patch

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is