Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Tim Angus <tim@ngus.net>
To: ffmpeg-devel@ffmpeg.org
Cc: Tim Angus <tim@ngus.net>
Subject: [FFmpeg-devel] [PATCH 0/1] Handle ASS format subtitle encoding ambiguity
Date: Fri, 27 Jan 2023 17:20:46 +0000
Message-ID: <20230127172047.1024276-1-tim@ngus.net> (raw)

Some matroska files embed ASS format subtitles. The header for said
subtitles include the header for the subtitle stream in the "codec
private data" section. It's not clear whether the last byte of this data
is supposed to be 0, i.e. a null terminator for the string data. Among
other tools, older versions of Handbrake do include the null terminator
and there are many files out in the wild created using Handbrake. Using
ffmpeg to extract subtitles for such a file, this header is copied
directly to the output file, including the null terminator, if it was
present. This results in a file in which there is a null terminator
after the header, but preceeding the actual content of the subtitle
file. Obviously this is not correct.

As a data point, of the ~600 mkvs I have locally, 22 of them have ASS
subtitles, and of them 20 include the null terminator, so it doesn't appear to
be a rare phenomenon.

As another data point, the tool mkvextract from mkvtoolnix avoids the ambiguity
by first assuming that the source buffer is *not* null terminated, and then
manually adding a (possibly second) null terminator. The buffer is then
interpreted as a null terminated string and processed that way.
(https://gitlab.com/mbunkus/mkvtoolnix/-/blob/main/src/extract/xtr_textsubs.cpp#L117)

My change here refactors the way the output file is created, by treating
the source buffer as a string. I posted an earlier iteration of this
patch to the list that dealt with the extra null explicitly, but
following some #ffmpeg-devel discussion, I suggested I rework it to work
more like how mkvtoolnix does things, and this was positively received,
so here we are. 

FATE succeeds as there are no mkvs in the suite that have ASS subtitles
embedded.

A small test file that exhibits the problem may be found here:
https://0x0.st/oFTT.mkv

Tim Angus (1):
  avformat/assenc: avoid incorrect copy of null terminator

 libavformat/assenc.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

-- 
2.25.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

             reply	other threads:[~2023-01-27 17:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-27 17:20 Tim Angus [this message]
2023-01-27 17:20 ` [FFmpeg-devel] [PATCH 1/1] avformat/assenc: avoid incorrect copy of null terminator Tim Angus
2023-02-08 15:00   ` Tim Angus
  -- strict thread matches above, loose matches on Subject: below --
2023-01-18 14:31 [FFmpeg-devel] [PATCH 0/1] Handle ASS format subtitle encoding ambiguity Tim Angus
2023-01-21 19:50 ` Tim Angus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230127172047.1024276-1-tim@ngus.net \
    --to=tim@ngus.net \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git