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

CMITT 1.3.0 ST0604.3-7 non compliance #102

Open
bradh opened this issue Apr 15, 2020 · 3 comments
Open

CMITT 1.3.0 ST0604.3-7 non compliance #102

bradh opened this issue Apr 15, 2020 · 3 comments

Comments

@bradh
Copy link
Collaborator

bradh commented Apr 15, 2020

Describe the bug
On some self-generated code that attempts to encode the image frame, CMITT flags a non-compliance: "ST 0604.3-07: All video frames must contain a timestamp. "

To Reproduce
Steps to reproduce the behavior:

  1. I'm using output.addVideoFrame(new VideoFrame(image, pts)); where image is a BufferedImage, and pts is the same timestamp I'm passing to the KLV build.
  2. Build video
  3. Test in CMITT 1.3.0
  4. See flagged issue.

Expected behavior
Green tick in CMITT.

Screenshots
N/A

Configuration (please complete the following information):
N/A.

Additional context
api/src/main/java/org/jmisb/api/video/VideoOutput.java has a couple of methods that are probably related.

@bradh
Copy link
Collaborator Author

bradh commented Apr 19, 2020

Some more detailed reading suggests its probably looking for a video packet level metadata entry.

An example that does include it is shown in https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/uploads/addca4b4d4d9dca44456d0a5bcf6b8ed/klv_metadata_test_sync.ts

$ ffprobe -select_streams v -show_packets -show_data klv_metadata_test_sync.ts 
[PACKET]
codec_type=video
stream_index=1
pts=2906540998
pts_time=32294.899978
dts=2906540998
dts_time=32294.899978
duration=3003
duration_time=0.033367
convergence_duration=N/A
convergence_duration_time=N/A
size=27817
pos=564
flags=K_
[SIDE_DATA]
side_data_type=MPEGTS Stream ID
[/SIDE_DATA]
data=
00000000: 0000 0001 0910 0000 0001 674d 4029 f201  ..........gM@)..
00000010: 687b 7fe0 0400 0362 0000 07d2 0001 d4c1  h{.....b........
00000020: 3e30 6490 0000 0001 68eb cf20 0000 0001  >0d.....h.. ....
00000030: 0601 0609 0c1b 1809 0080 0000 0001 0605  ................
00000040: 1c4d 4953 506d 6963 726f 7365 6374 696d  .MISPmicrosectim
00000050: 651f 0005 ff60 28ff 6bca ff14 6780 0000  e....`(.k...g...
00000060: 0001 6588 8404 ff86 a2ae 91a2 4183 5352  ..e.........A.SR

The key part is that "MISPmicrosectime" key, followed by a one byte flag and then a timestamp: 0x00 0x05 0xff 0x60 0x28 0xff 0x6b 0xca 0xff 0x14 0x67 where the 0xff are pad bytes.
That looks a bit different between H.264 and H.265, but

H.264/AVC provides a Supplemental Enhancement Information (SEI) message – designated the
user_data_unregistered SEI message field. The user_data_unregistered SEI message consists of
two subfields: the uuid_iso_iec_11578, which is a 16-byte UUID, and the
user_data_payload_byte, which is a variable length field. The uuid_iso_iec_11578 is set to the
16-byte Precision Time Stamp Identifier. The user_data_payload_byte is set to the 12-byte
combination Time Status and Modified Precision Time Stamp (Bytes 17-28 of Table 2 with Byte
17 transmitted first).

(Note that requirement ST0604.3-7 was deprecated somewhere along the line to ST0604.6, but there are equivalent requirements in the newer versions).

@bradh
Copy link
Collaborator Author

bradh commented Apr 19, 2020

I also found https://medium.com/@kydare/decoding-unregistered-user-data-sei-in-ffmpeg-5de68b1cc9df but I don't yet see how to use any of that with the java bindings we have.

@bradh
Copy link
Collaborator Author

bradh commented Aug 27, 2020

OK. Using a current git version of ffmpeg from today, I managed to generate:

Precision Time Stamp Identifier MISPmicrosectime
Time Status: 0x1f
Modified Precision Time Stamp: [0x00 0x05] 0xff [0x21 0x7e] 0xff [0x2a 0x3d] 0xff [0xa9 0xfc]
Precision Time Stamp Identifier MISPmicrosectime
Time Status: 0x1f
Modified Precision Time Stamp: [0x00 0x05] 0xff [0x21 0x7e] 0xff [0x2a 0x3e] 0xff [0xae 0x62]
Precision Time Stamp Identifier MISPmicrosectime
Time Status: 0x1f
Modified Precision Time Stamp: [0x00 0x05] 0xff [0x21 0x7e] 0xff [0x2a 0x3f] 0xff [0x30 0xa7]

(more of that)

The code to produce that looks like:

diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c7b2764270..cd83791d82 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1294,9 +1294,29 @@ static int h264_export_frame_props(H264Context *h)
         H264SEIUnregistered *unreg = &h->sei.unregistered;
 
         if (unreg->buf_ref[i]) {
+            char *precisionTimeStampIdentifier;
             AVFrameSideData *sd = av_frame_new_side_data_from_buf(out,
                     AV_FRAME_DATA_SEI_UNREGISTERED,
                     unreg->buf_ref[i]);
+
+            // printf("unregistered %u\n", unreg->buf_ref[i]->size);
+            precisionTimeStampIdentifier = av_strndup(unreg->buf_ref[i]->data, 16);
+            printf("Precision Time Stamp Identifier %s\n", precisionTimeStampIdentifier);
+            printf("Time Status: 0x%02x\n", unreg->buf_ref[i]->data[16]);
+            printf("Modified Precision Time Stamp: [0x%02x 0x%02x] 0x%02x [0x%02x 0x%02x] 0x%02x [0x%02x 0x%02x] 0x%02x [0x%02x 0x%02x]\n",
+                unreg->buf_ref[i]->data[17],
+                unreg->buf_ref[i]->data[18],
+                unreg->buf_ref[i]->data[19],
+                unreg->buf_ref[i]->data[20],
+                unreg->buf_ref[i]->data[21],
+                unreg->buf_ref[i]->data[22],
+                unreg->buf_ref[i]->data[23],
+                unreg->buf_ref[i]->data[24],
+                unreg->buf_ref[i]->data[25],
+                unreg->buf_ref[i]->data[26],
+                unreg->buf_ref[i]->data[27]
+                );
+            av_free(precisionTimeStampIdentifier);
             if (!sd)
                 av_buffer_unref(&unreg->buf_ref[i]);
             unreg->buf_ref[i] = NULL;

Clearly we don't want to modify ffmpeg for the jMISB usage, but it looks like we can do something like:

diff --git a/api/src/main/java/org/jmisb/api/video/VideoDecodeThread.java b/api/src/main/java/org/jmisb/api/video/VideoDecodeThread.java
index fbfb029..6276c36 100644
--- a/api/src/main/java/org/jmisb/api/video/VideoDecodeThread.java
+++ b/api/src/main/java/org/jmisb/api/video/VideoDecodeThread.java
@@ -9,10 +9,12 @@ import static org.bytedeco.ffmpeg.global.avcodec.avcodec_open2;
 import static org.bytedeco.ffmpeg.global.avcodec.avcodec_parameters_to_context;
 import static org.bytedeco.ffmpeg.global.avcodec.avcodec_receive_frame;
 import static org.bytedeco.ffmpeg.global.avcodec.avcodec_send_packet;
+import static org.bytedeco.ffmpeg.global.avutil.AV_FRAME_DATA_SEI_UNREGISTERED;
 import static org.bytedeco.ffmpeg.global.avutil.AV_PIX_FMT_BGR24;
 import static org.bytedeco.ffmpeg.global.avutil.av_dict_free;
 import static org.bytedeco.ffmpeg.global.avutil.av_frame_alloc;
 import static org.bytedeco.ffmpeg.global.avutil.av_frame_free;
+import static org.bytedeco.ffmpeg.global.avutil.av_frame_get_side_data;
 import static org.bytedeco.ffmpeg.global.avutil.av_image_fill_arrays;
 import static org.bytedeco.ffmpeg.global.avutil.av_image_get_buffer_size;
 import static org.bytedeco.ffmpeg.global.avutil.av_malloc;
@@ -31,6 +33,7 @@ import org.bytedeco.ffmpeg.avcodec.AVPacket;
 import org.bytedeco.ffmpeg.avformat.AVStream;
 import org.bytedeco.ffmpeg.avutil.AVDictionary;
 import org.bytedeco.ffmpeg.avutil.AVFrame;
+import org.bytedeco.ffmpeg.avutil.AVFrameSideData;
 import org.bytedeco.ffmpeg.swscale.SwsContext;
 import org.bytedeco.javacpp.*;
 import org.jmisb.core.video.FfmpegUtils;
@@ -166,6 +169,9 @@ class VideoDecodeThread extends ProcessingThread {
                         double pts = packet.pts() * av_q2d(videoStream.time_base());
                         // logger.debug("Video PTS = " + pts);
 
+                        AVFrameSideData sideData =
+                                av_frame_get_side_data(avFrame, AV_FRAME_DATA_SEI_UNREGISTERED);
+
                         // Convert image from native pixel format to BGR24
                         sws_scale(
                                 swsContext,

We can then pull the bits we want out of the side data.

However there are going to be delays. This code didn't make it in to 4.3. So its unlikely to appear until 4.4, which I think is probably going to be early 2021. Then we need presets to catch up with a release.

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

No branches or pull requests

1 participant