Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch: avformat/webvttdec: Ignore REGION and STYLE chunks #26

Merged
merged 1 commit into from
Jun 22, 2022

Conversation

tpikonen
Copy link

This patch fixes WebVTT subtitle decoding.

@tpikonen tpikonen marked this pull request as draft June 20, 2022 10:14
@tpikonen
Copy link
Author

I did not check if this actually builds, so setting as draft for now.

@pukkandan
Copy link
Member

pukkandan commented Jun 20, 2022

@nihil-admirari
Copy link
Collaborator

  1. Too many unrelated changes, please rebase on top of master. Unfortunately, because of continuous automatic rebase, this step may have to be repeated multiple times.
  2. Please fill in the README.
  3. You patch is present in the mailing list: https://ffmpeg.org/pipermail/ffmpeg-devel/2022-May/296353.html, but I was unable to find it in Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?q=webvttdec. Probably makes sense to resubmit.
  4. Have you tried to ping the devs in IRC?

@tpikonen
Copy link
Author

Thanks @nihil-admirari, I did not know about the ffmpeg patchwork before. In the patchwork link you provided, I counted 4 patches claiming to fix the same problem as this patch, some of them several years old. I don't think adding a fifth one helps.

I don't use these builds myself, so I'm not going to do any more work to get this patch in, but please do include it, if you find it useful.

@nihil-admirari
Copy link
Collaborator

I don't use these builds myself, so I'm not going to do any more work to get this patch in, but please do include it, if you find it useful.
Ok, I'll do.

I counted 4 patches claiming to fix the same problem as this patch, some of them several years old. I don't think adding a fifth one helps.
Can you please provide the links to these patches? I'll ask FFmpeg devs why they haven't been merged yet.

@tpikonen
Copy link
Author

Can you please provide the links to these patches? I'll ask FFmpeg devs why they haven't been merged yet.

HTH

@nihil-admirari
Copy link
Collaborator

https://patchwork.ffmpeg.org/project/ffmpeg/patch/CA+_k4RJ1CNriPbhxxzLf1O6V1VuP2OXch3E8LqOsMNSRUyEnTw@mail.gmail.com/ is the only patch with tests; unfortunately they fail, and that's probably why it hasn't been merged.

@gamer191
Copy link

gamer191 commented Jun 22, 2022

https://patchwork.ffmpeg.org/project/ffmpeg/patch/CA+_k4RJ1CNriPbhxxzLf1O6V1VuP2OXch3E8LqOsMNSRUyEnTw@mail.gmail.com/ is the only patch with tests

wdym? They all have tests, and the tests for the first two patches passed. The third one's tests are stuck on pending, and the 4th one fails (as you know)

EDIT: never mind, I misunderstood you

@gamer191 gamer191 mentioned this pull request Jun 22, 2022
@pukkandan
Copy link
Member

@nihil-admirari Thanks for dealing with this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants