search

Kitware / fletch

Computer Vision Software Development Environment
63 stars 54 forks source link

Patch FFmpeg to write unregistered SEI user data #721

Closed daniel-riehm closed 1 year ago

daniel-riehm commented 1 year ago

MISB ST0604 states that MISP frame timestamps should be put in the "SEI unregistered user data" field of H.264 and H.265 video streams. Our most recent version of FFmpeg, 4.4.1, implements reading of these fields but not writing. This PR pulls in more recent feature additions from the public FFmpeg repository, adding only the code necessary to write these fields (or, more accurately, to tell x264, x265, and nvenc to write these fields).

daniel-riehm commented 1 year ago

jenkins test this please

kwcvrobot commented 1 year ago

Fletch KWIVER against Fletch Linux build succeeded.

kwcvrobot commented 1 year ago

Fletch KWIVER against Fletch Centos build succeeded.

kwcvrobot commented 1 year ago

Fletch KWIVER against Fletch Windows build succeeded.

chetnieter commented 1 year ago

It looks there has been several minor (4.4.2 & 4.4.3) and major (5.0. and 5.1.) release of FFMpeg since this PR was open. Should we consider upgrading ffmpeg if the fix has made it into one of these releases?

daniel-riehm commented 1 year ago

@chetnieter Sure, we can look into that in another PR, but it would likely come with significant issues, as there are a number of existing patches we have for 4.4.1 that we would have to port over to 5.x. And moving to a new major version likely comes with some interface / behavior changes as well, which we would have to deal with in KWIVER. Are there any objections to merging this for the interim?

chetnieter commented 1 year ago

@chetnieter Sure, we can look into that in another PR, but it would likely come with significant issues, as there are a number of existing patches we have for 4.4.1 that we would have to port over to 5.x. And moving to a new major version likely comes with some interface / behavior changes as well, which we would have to deal with in KWIVER. Are there any objections to merging this for the interim?

Since we would have to bump up a major version patching it for now is fine.