| Message ID | MWHPR13MB1678D49B7680D92BC1EC159DBA630@MWHPR13MB1678.namprd13.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 20e8be0c20c7b51964fa4d317073bd36b983eb55 |
| Headers | show |
On 1/6/2017 8:33 PM, Soft Works wrote: > Revision #4: Don't create CRC32 for preliminary headers > > The following three commits created a regression by writing initially > invalid mkv headers: > > 650e17d88b63b5aca6e0a43483e89e64b0f7d2dd avformat/matroskaenc: write a > CRC32 element on Tags > 3bcadf822711720ff0f8d14db71ae47cdf97e652 avformat/matroskaenc: write a > CRC32 element on Info > ee888cfbe777cd2916a3548c750e433ab8f8e6a5 avformat/matroskaenc: postpone > writing the Tracks master > > Symptoms: > > - You can no longer playback a file that is still processed by ffmpeg, > e.g. VLC fails playback > - You can no longer stream a file to a client while if is still being > processed > - Various diagnosing tools show header errors or incomplete headers > (e.g. ffprobe, mediainfo, mkvalidator) > > Note: The symptoms do not apply to completed files or ffmpeg runs that > were interrupted with 'q' > > Cause: > > The mentioned commits made changes in a way that some header elements > are only partially written in > mkv_write_header, leaving the header in an invalid state. Only in > mkv_write_trailer, these elements > are finished correctly, but that does only occur at the end of the > process. > > Regression: > > Before these commits were applied, mkv headers have always been valid, > even before completion of ffmpeg. > This has worked reliably over many versions of ffmpeg, to it was an > obvious regression. > > Bugtracker: > > This issue has been recorded as #5977 which is resolved by this patch > > Patch: > > The patch adds a new function 'end_ebml_master_crc32_preliminary' that > preliminarily finishes the ebl > element without destroying the buffer. The buffer can be used to update > the ebml element later during > mkv_write_trailer. But most important: mkv_write_header finishes with a > valid mkv header again. > --- > libavformat/matroskaenc.c | 22 +++++++++++++++++++--- > 1 file changed, 19 insertions(+), 3 deletions(-) > > diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c > index 827d755..dd8ff8e 100644 > --- a/libavformat/matroskaenc.c > +++ b/libavformat/matroskaenc.c > @@ -367,6 +367,22 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, Matrosk > *dyn_cp = NULL; > } > > +/** > +* Complete ebml master whithout destroying the buffer, allowing for later updates > +*/ > +static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, > + ebml_master master) > +{ > + if (pb->seekable) { > + > + uint8_t *buf; > + int size = avio_get_dyn_buf(*dyn_cp, &buf); > + > + avio_write(pb, buf, size); > + end_ebml_master(pb, master); > + } > +} > + > static void put_xiph_size(AVIOContext *pb, int size) > { > ffio_fill(pb, 255, size / 255); > @@ -1309,7 +1325,7 @@ static int mkv_write_tracks(AVFormatContext *s) > } > > if (pb->seekable && !mkv->is_live) > - put_ebml_void(pb, avio_tell(mkv->tracks_bc)); > + end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); > else > end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); > > @@ -1554,7 +1570,7 @@ static int mkv_write_tags(AVFormatContext *s) > > if (mkv->tags.pos) { > if (s->pb->seekable && !mkv->is_live) > - put_ebml_void(s->pb, avio_tell(mkv->tags_bc)); > + end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv, mkv->tags); > else > end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, mkv->tags); > } > @@ -1811,7 +1827,7 @@ static int mkv_write_header(AVFormatContext *s) > } > } > if (s->pb->seekable && !mkv->is_live) > - put_ebml_void(s->pb, avio_tell(pb)); > + end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv, mkv->info); > else > end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info); > pb = s->pb; Pushed, thanks.
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c index 827d755..dd8ff8e 100644 --- a/libavformat/matroskaenc.c +++ b/libavformat/matroskaenc.c @@ -367,6 +367,22 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp, Matrosk *dyn_cp = NULL; } +/** +* Complete ebml master whithout destroying the buffer, allowing for later updates +*/ +static void end_ebml_master_crc32_preliminary(AVIOContext *pb, AVIOContext **dyn_cp, MatroskaMuxContext *mkv, + ebml_master master) +{ + if (pb->seekable) { + + uint8_t *buf; + int size = avio_get_dyn_buf(*dyn_cp, &buf); + + avio_write(pb, buf, size); + end_ebml_master(pb, master); + } +} + static void put_xiph_size(AVIOContext *pb, int size) { ffio_fill(pb, 255, size / 255); @@ -1309,7 +1325,7 @@ static int mkv_write_tracks(AVFormatContext *s) } if (pb->seekable && !mkv->is_live) - put_ebml_void(pb, avio_tell(mkv->tracks_bc)); + end_ebml_master_crc32_preliminary(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); else end_ebml_master_crc32(pb, &mkv->tracks_bc, mkv, mkv->tracks_master); @@ -1554,7 +1570,7 @@ static int mkv_write_tags(AVFormatContext *s) if (mkv->tags.pos) { if (s->pb->seekable && !mkv->is_live) - put_ebml_void(s->pb, avio_tell(mkv->tags_bc)); + end_ebml_master_crc32_preliminary(s->pb, &mkv->tags_bc, mkv, mkv->tags); else end_ebml_master_crc32(s->pb, &mkv->tags_bc, mkv, mkv->tags); } @@ -1811,7 +1827,7 @@ static int mkv_write_header(AVFormatContext *s) } } if (s->pb->seekable && !mkv->is_live) - put_ebml_void(s->pb, avio_tell(pb)); + end_ebml_master_crc32_preliminary(s->pb, &mkv->info_bc, mkv, mkv->info); else end_ebml_master_crc32(s->pb, &mkv->info_bc, mkv, mkv->info); pb = s->pb;