[FFmpeg-devel] [PATCH] libavcodec/ccaption_dec.c: Fixed indentation overwriting text in the cea608 caption decoder.

[Levi Dooley]

Yeah, I am looking into the test failure now. Sorry about that. I did run
"make fate" prior to submitting the patch, but I missed the fact that I
needed the sample files first, so it looked like it passed all the tests at
the time. I have the sample files now, and I am correctly seeing the test
failures locally. The test failures did point out a bug in the patch, and I
am fixing it now. I will submit an updated patch as soon as that is done.

Thanks,
Levi

On Wed, Jan 27, 2021 at 3:19 PM Andreas Rheinhardt <
andreas.rheinhardt at gmail.com> wrote:

> Aman Karmani:
> > On Mon, Jan 25, 2021 at 3:16 PM Levi Dooley <i.am.stickfigure at gmail.com>
> > wrote:
> >
> >> There was an assumption in the existing code that indentation would not
> >> occur more than once on the same row.
> >> This was a bad assumption. There are examples of 608 streams which call
> >> handle_pac multiple times on the same row with different indentation.
> >> As the code was before this change, the new indentation would overwrite
> >> existing text with spaces.
> >> These changes make indentation skip over columns instead. Text gets
> cleared
> >> with spaces on handle_edm.
> >> Instead of relying on the null character, trailing spaces are trimmed
> off
> >> the end of a row.
> >> This is necessary so that a null character doesn't end up between two
> >> words.
> >>
> >> Signed-off-by: Levi Dooley <i.am.stickfigure at gmail.com>
> >>
> >> Here's a link to a sample file that will reproduce this issue.
> >>
> >>
> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip.ts
> >>
> >> The issue can be reproduced by running the following command:
> >>
> >>> ffmpeg -f lavfi -i "movie=cleveland-clip.ts[out0+subcc]" -map s
> >>> cleveland-clip.ass
> >>
> >>
> >> I've gone ahead and ran this command both before and after my code
> changes.
> >> The following output files demonstrate that there are some clear cases
> of
> >> missing words or sentences in the beforepatch file, and it is entirely
> >> fixed by this patch in the afterpatch file.
> >>
> >> Before this patch:
> >>
> >>
> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-beforepatch.ass
> >>
> >> After this patch:
> >>
> >>
> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/cleveland-clip-afterpatch.ass
> >>
> >> And here is the full sample video in case anyone wants to play around
> with
> >> a larger example with many more caption errors. The above video sample
> >> "cleveland-clip.ts" is just a 60 second clip of the following.
> >>
> >>
> https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/ffmpeg-caption-issue/The%20Cleveland%20Show%20-%20%28Brown%20Magic%29-2018-12-14-0.ts
> >>
> >> The full patch file is attached to this email.
> >
> >
> > Patch looks reasonable to me. Thanks for sharing the sample and
> > before/after output.
> >
> > Will commit in a couple days if there are no additional comments.
> >
> > Aman
> >
> This patch breaks FATE [1]; this needs to be analyzed. If the changes
> are ok, the ref files need to be updated in the same patch.
>
> - Andreas
>
> [1]:
>
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210126215003.178259-1-levi@snapstream.com/
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".