mbox series

[FFmpeg-devel,v4,0/4] Fix some active sequences in subtitles

Message ID 20240219214227.19814-1-oneric@oneric.de
Headers show
Series Fix some active sequences in subtitles | expand

Message

Oneric Feb. 19, 2024, 9:42 p.m. UTC
Changes from v3:
 - None.  Just rebased ontop of master to allow
   Patchwork to properly process the series now
   that 99d33cc661fbd04e8657831 was merged.

Changes from v2:
 - rebased ontop of the recently pushed eol normalisation.
   As a result no more CRLFs in here and Patchwork should be happy
 - added a fourth cosmetic commit adjusting
   explicit linebreaks to the new normalisation

Changes from v1:
 - ff_ass_bprint_text_event now only inserts a word-joiner
   if there isn’t already one anyway
 - added a third commit improving the handling of
   curly brackets for standard ASS renderers

Oneric (4):
  avcodec/webvttdec: honour bidi marks
  avcodec/{ass,webvttdec}: fix handling of backslashes
  avcodec/{ass,webvttdec}: more portable curly brace escapes
  avocdec/ass: simplify linebreaks

 libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
 libavcodec/webvttdec.c     |  4 ++--
 tests/ref/fate/sub-webvtt  |  2 +-
 tests/ref/fate/sub-webvtt2 |  2 +-
 4 files changed, 33 insertions(+), 22 deletions(-)

Comments

Oneric Feb. 26, 2024, 10:26 p.m. UTC | #1
ping

I forgot to again add a description to v4 so:
These small patches fix several mis-conversions for subtitles
(and additionally one cde cleanup in avcodec/ass.c):

 - WebVTT files no longer get their BiDi marks stripped
   (which mangled BiDi text)
 - backslashes are no longer duplicated for no reason
 - ffmpeg’s attempt at escaping curly braces no longer
   severly breaks in standard ASS renderers

v1 was posted over 2 years ago, would be great
if someone could finally look at this


On Mon, Feb 19, 2024 at 22:42:23 +0100, Oneric wrote:
> Changes from v3:
>  - None.  Just rebased ontop of master to allow
>    Patchwork to properly process the series now
>    that 99d33cc661fbd04e8657831 was merged.
> 
> Changes from v2:
>  - rebased ontop of the recently pushed eol normalisation.
>    As a result no more CRLFs in here and Patchwork should be happy
>  - added a fourth cosmetic commit adjusting
>    explicit linebreaks to the new normalisation
> 
> Changes from v1:
>  - ff_ass_bprint_text_event now only inserts a word-joiner
>    if there isn’t already one anyway
>  - added a third commit improving the handling of
>    curly brackets for standard ASS renderers
> 
> Oneric (4):
>   avcodec/webvttdec: honour bidi marks
>   avcodec/{ass,webvttdec}: fix handling of backslashes
>   avcodec/{ass,webvttdec}: more portable curly brace escapes
>   avocdec/ass: simplify linebreaks
> 
>  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
>  libavcodec/webvttdec.c     |  4 ++--
>  tests/ref/fate/sub-webvtt  |  2 +-
>  tests/ref/fate/sub-webvtt2 |  2 +-
>  4 files changed, 33 insertions(+), 22 deletions(-)
Marth64 March 19, 2024, 8:59 p.m. UTC | #2
Hi Oneric,

I was able to validate the whole set. Here are the tests I did ...
(1) test conversion from vtt->ass with bidi mark and visually check display
-- pass
(2) test slash escape fix with a string e.g. \\Nancy, verify broken before
and fixed by patch -- pass
(3) test curly brace escape fix doesn't break SRT titles with ass
positioning tags -- pass
(4) compare sha256sum before/after results for line breaks change -- pass

LGTM from a validation perspective on a Linux machine.
Marth64 March 26, 2024, 4:01 p.m. UTC | #3
Ping on this set authored by Oneric as well.
Oneric April 3, 2024, 7:50 p.m. UTC | #4
On Mon, 19 Feb 2024 Oneric wrote:
>   avcodec/webvttdec: honour bidi marks
>   avcodec/{ass,webvttdec}: fix handling of backslashes
>   avcodec/{ass,webvttdec}: more portable curly brace escapes
>   avocdec/ass: simplify linebreaks
>
>  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
>  libavcodec/webvttdec.c     |  4 ++--
>  tests/ref/fate/sub-webvtt  |  2 +-
>  tests/ref/fate/sub-webvtt2 |  2 +-
>  4 files changed, 33 insertions(+), 22 deletions(-)

On Tue, Mar 26, 2024 at 11:01:40 -0500, Marth64 wrote:
> Ping on this set authored by Oneric as well.

ping again
Stefano Sabatini April 6, 2024, 7:27 a.m. UTC | #5
On date Wednesday 2024-04-03 21:50:29 +0200, Oneric wrote:
> On Mon, 19 Feb 2024 Oneric wrote:
> >   avcodec/webvttdec: honour bidi marks
> >   avcodec/{ass,webvttdec}: fix handling of backslashes
> >   avcodec/{ass,webvttdec}: more portable curly brace escapes
> >   avocdec/ass: simplify linebreaks
> >
> >  libavcodec/ass.c           | 47 +++++++++++++++++++++++---------------
> >  libavcodec/webvttdec.c     |  4 ++--
> >  tests/ref/fate/sub-webvtt  |  2 +-
> >  tests/ref/fate/sub-webvtt2 |  2 +-
> >  4 files changed, 33 insertions(+), 22 deletions(-)
> 
> On Tue, Mar 26, 2024 at 11:01:40 -0500, Marth64 wrote:
> > Ping on this set authored by Oneric as well.
> 
> ping again

will apply this soon, thanks