[06/14] movenc: Get rid of a hack for updating the dvc1 atom

Message ID 1419862828-30060-6-git-send-email-martin@martin.st
State Committed
Commit b3b0b35db2f3b61bf2f0f4fa85f5b6267d83c8fe
Headers show

Commit Message

Martin Storsjö Dec. 29, 2014, 2:20 p.m.
Use the more generic approach with the delay_moov flag, instead of
having a update mechanism specific to this one single atom.
---
 libavformat/movenc.c | 20 ++++----------------
 libavformat/movenc.h |  1 -
 2 files changed, 4 insertions(+), 17 deletions(-)

Comments

Derek Buitenhuis Dec. 29, 2014, 5:13 p.m. | #1
On 12/29/2014 2:20 PM, Martin Storsjö wrote:
> Use the more generic approach with the delay_moov flag, instead of
> having a update mechanism specific to this one single atom.
> ---
>  libavformat/movenc.c | 20 ++++----------------
>  libavformat/movenc.h |  1 -
>  2 files changed, 4 insertions(+), 17 deletions(-)

[...]

> +        av_log(NULL, AV_LOG_WARNING,
> +               "moov atom written before any packets, unable to write correct "
> +               "dvc1 atom. Set the delay_moov flag to fix this.\n");

Won't this result in broken file?

- Derek
Martin Storsjö Dec. 29, 2014, 5:28 p.m. | #2
On Mon, 29 Dec 2014, Derek Buitenhuis wrote:

> On 12/29/2014 2:20 PM, Martin Storsjö wrote:
>> Use the more generic approach with the delay_moov flag, instead of
>> having a update mechanism specific to this one single atom.
>> ---
>>  libavformat/movenc.c | 20 ++++----------------
>>  libavformat/movenc.h |  1 -
>>  2 files changed, 4 insertions(+), 17 deletions(-)
>
> [...]
>
>> +        av_log(NULL, AV_LOG_WARNING,
>> +               "moov atom written before any packets, unable to write correct "
>> +               "dvc1 atom. Set the delay_moov flag to fix this.\n");
>
> Won't this result in broken file?

Probably not too broken - the few bits we can't deduce without looking at 
the individual packets are set to sane defaults. There are bits for 
indicating "this stream won't contain multiple sequence headers" or "this 
stream won't contain multiple entrypoints" (I'm not too familiar with VC1 
so I don't remember exactly what this means), and if in doubt we set them 
to 0, saying that the stream might contain that.

Our own demuxer doesn't look at these fields at all.

All in all I think it's easier to just go this way, than to keep an extra 
hack just for the hypothetical case of people writing vc1 with our mp4 
muxer, fragmented with empty_moov. (The only real case of using vc1 in mp4 
would be for smooth streaming, and in those cases, nothing of the moov 
atom is even transmitted over the wire.)

// Martin
Derek Buitenhuis Dec. 29, 2014, 5:33 p.m. | #3
On 12/29/2014 5:28 PM, Martin Storsjö wrote:
>> Won't this result in broken file?
> 
> Probably not too broken - the few bits we can't deduce without looking at 
> the individual packets are set to sane defaults. There are bits for 
> indicating "this stream won't contain multiple sequence headers" or "this 
> stream won't contain multiple entrypoints" (I'm not too familiar with VC1 
> so I don't remember exactly what this means), and if in doubt we set them 
> to 0, saying that the stream might contain that.

Well, OK. The whole setup sounds a bit silly here, but I guess we can't
exactly go back in time and slap people.

> Our own demuxer doesn't look at these fields at all.

Shocking.

> All in all I think it's easier to just go this way, than to keep an extra 
> hack just for the hypothetical case of people writing vc1 with our mp4 
> muxer, fragmented with empty_moov. (The only real case of using vc1 in mp4 
> would be for smooth streaming, and in those cases, nothing of the moov 
> atom is even transmitted over the wire.)

I, of course, agree.

- Derek

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index fe95b84..27421ba 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -447,9 +447,11 @@  static int mov_write_dvc1_structs(MOVTrack *track, uint8_t *buf)
 
     if (track->start_dts == AV_NOPTS_VALUE) {
         /* No packets written yet, vc1_info isn't authoritative yet. */
-        /* Assume inline sequence and entry headers. This will be
-         * overwritten at the end if the file is seekable. */
+        /* Assume inline sequence and entry headers. */
         packet_seq = packet_entry = 1;
+        av_log(NULL, AV_LOG_WARNING,
+               "moov atom written before any packets, unable to write correct "
+               "dvc1 atom. Set the delay_moov flag to fix this.\n");
     }
 
     unescaped = av_mallocz(track->vos_len + FF_INPUT_BUFFER_PADDING_SIZE);
@@ -525,7 +527,6 @@  static int mov_write_dvc1_tag(AVIOContext *pb, MOVTrack *track)
 
     avio_wb32(pb, track->vos_len + 8 + sizeof(buf));
     ffio_wfourcc(pb, "dvc1");
-    track->vc1_info.struct_offset = avio_tell(pb);
     avio_write(pb, buf, sizeof(buf));
     avio_write(pb, track->vos_data, track->vos_len);
 
@@ -4088,19 +4089,6 @@  static int mov_write_trailer(AVFormatContext *s)
         }
     }
 
-    for (i = 0; i < mov->nb_streams; i++) {
-        if (mov->flags & FF_MOV_FLAG_FRAGMENT &&
-            mov->tracks[i].vc1_info.struct_offset && s->pb->seekable) {
-            int64_t off = avio_tell(pb);
-            uint8_t buf[7];
-            if (mov_write_dvc1_structs(&mov->tracks[i], buf) >= 0) {
-                avio_seek(pb, mov->tracks[i].vc1_info.struct_offset, SEEK_SET);
-                avio_write(pb, buf, 7);
-                avio_seek(pb, off, SEEK_SET);
-            }
-        }
-    }
-
 error:
     mov_free(s);
 
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 8b1084e..682820e 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -129,7 +129,6 @@  typedef struct MOVTrack {
     unsigned    frag_info_capacity;
 
     struct {
-        int64_t struct_offset;
         int     first_packet_seq;
         int     first_packet_entry;
         int     packet_seq;