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

libavformat/matroskadec: set fixed duration for subtitles #48

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

enen92
Copy link

@enen92 enen92 commented Jan 9, 2023

The matroska specification states the start time and duration of
subtitle entries are encoded in the block TimeStamp
and BlockDuration. Furthermore, for all subtitle formats except
S_HDMV/PGS the BlockDuration must always be defined and have an absolute
value even if it is simply 0. ffmpeg assumes that a duration of 0 means
the duration is still unknown and tries to adjust based on the next packet
pts. This is wrong for all formats except S_HDMV/PGS.
Since changing the semantics of duration 0 is not an option (touches too
many parts of the code) this change introduces AV_PKT_FLAG_FIXED_DURATION flag which
decoders might use to flag the duration of a given packet should not be
changed.

Signed-off-by: Miguel Borges de Freitas [email protected]


This is my attempt at fixing https://trac.ffmpeg.org/ticket/10135
Initially reported to Kodi in xbmc/xbmc#21625

cc: Miguel Borges de Freitas [email protected]

@tmm1
Copy link

tmm1 commented Jan 9, 2023

/approve

@enen92 enen92 force-pushed the ass_mkv_fixed_subs branch 2 times, most recently from b35adad to 3752b85 Compare January 10, 2023 11:18
@ffmpeg-codebot
Copy link

There is an issue in commit 3752b85:
Please wrap lines in the body of the commit message between 60 and 72 characters.

@enen92 enen92 force-pushed the ass_mkv_fixed_subs branch 2 times, most recently from 30cd632 to 76ed24e Compare January 10, 2023 11:56
The matroska specification states the start time and duration of
subtitle entries are encoded in the block TimeStamp
and BlockDuration. Furthermore, for all subtitle formats except
S_HDMV/PGS the BlockDuration must always be defined and have an
absolute value even if it is simply 0. ffmpeg assumes that a duration
of 0 means the duration is still unknown and tries to adjust based on
the next packet pts. This is wrong for all formats except S_HDMV/PGS.
Since changing the semantics of duration 0 is not an option (touches
too many parts of the code) this change introduces
AV_PKT_FLAG_FIXED_DURATION flag which decoders might use to flag the
duration of a given packet should not be changed.

Signed-off-by: Miguel Borges de Freitas <[email protected]>
@enen92 enen92 force-pushed the ass_mkv_fixed_subs branch from 76ed24e to 5985bf7 Compare January 10, 2023 13:10
Updates some of the test reference data to include the new
AV_PKT_FLAG_FIXED_DURATION (this is the case for most matroska
subtitles)

Signed-off-by: Miguel Borges de Freitas <[email protected]>
@enen92
Copy link
Author

enen92 commented Jan 10, 2023

/submit

@ffmpeg-codebot
Copy link

Error: User enen92 is not yet permitted to use the FFmpeg CodeBot

@softworkz
Copy link
Collaborator

/approve

@softworkz
Copy link
Collaborator

/allow

@ffmpeg-codebot
Copy link

User enen92 is now allowed to use the FFmpeg Code Bot.

WARNING: enen92 has no public email address set on GitHub

@softworkz
Copy link
Collaborator

softworkz commented Jan 10, 2023

/approve

@tmm1 LOL, you got me confused myself for a minute - it's allow, not approve :-)

@enen92
Copy link
Author

enen92 commented Jan 10, 2023

/submit

@ffmpeg-codebot
Copy link

Submitted as [email protected]

To fetch this version into FETCH_HEAD:

git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-48/enen92/ass_mkv_fixed_subs-v1

To fetch this version to local tag pr-ffstaging-48/enen92/ass_mkv_fixed_subs-v1:

git fetch --no-tags https://github.com/ffstaging/FFmpeg tag pr-ffstaging-48/enen92/ass_mkv_fixed_subs-v1

@ffmpeg-codebot
Copy link

On the FFmpeg mailing list, Miguel Borges de Freitas wrote (reply to this):

Hey guys,

Any feedback on this patch?

Regards

_______________________________________________
ffmpeg-devel mailing list
[email protected]
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
[email protected] with subject "unsubscribe".

@ffmpeg-codebot
Copy link

User Miguel Borges de Freitas <[email protected]> has been added to the cc: list.

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.

3 participants