[FFmpeg-devel] [PATCH] avformat/matroskaenc: Regression fix for invalid MKV headers

[Michael Niedermayer]

On Tue, Jan 03, 2017 at 11:49:07PM +0000, Soft Works wrote:
> 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/avio.h        | 12 ++++++++++++
>  libavformat/aviobuf.c     | 17 +++++++++++++++++
>  libavformat/matroskaenc.c | 34 +++++++++++++++++++++++++++++++---
>  3 files changed, 60 insertions(+), 3 deletions(-)

this patch breaks fate
make fate
...
--- ./tests/ref/fate/rgb24-mkv  2017-01-04 12:03:10.556691008 +0100
+++ tests/data/fate/rgb24-mkv   2017-01-04 13:45:50.260877087 +0100
@@ -1,5 +1,5 @@
-94cce0d7d5b14b4c86e74a1ca454c5aa *tests/data/fate/rgb24-mkv.matroska
-58361 tests/data/fate/rgb24-mkv.matroska
+4060a15b991c314120c51ae7e95958b9 *tests/data/fate/rgb24-mkv.matroska
+58734 tests/data/fate/rgb24-mkv.matroska
 #tb 0: 1/10
 #media_type 0: video
 #codec_id 0: rawvideo
Test rgb24-mkv failed. Look at tests/data/fate/rgb24-mkv.err for details.
make: *** [fate-rgb24-mkv] Error 1

> 
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index b1ce1d1..f2b9a6f 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h

changes to avio* should be in a seperate patch

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20170104/77699c2a/attachment.sig>