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.rheinha...@gmail.com> wrote: > Aman Karmani: > > On Mon, Jan 25, 2021 at 3:16 PM Levi Dooley <i.am.stickfig...@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.stickfig...@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-l...@snapstream.com/ > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
