After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 699302 - h264parse doesn't wait for complete output caps, leading to not-negotiated errors when remuxing
h264parse doesn't wait for complete output caps, leading to not-negotiated er...
Status: RESOLVED DUPLICATE of bug 646327
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 695206 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-04-30 12:31 UTC by Ilya Smelykh
Modified: 2013-06-07 10:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Issue stream (1.56 MB, application/x-extension-h264)
2013-04-30 12:36 UTC, Ilya Smelykh
  Details
Patch that fixes #699302 bug. (800 bytes, patch)
2013-05-06 15:48 UTC, Ilya Smelykh
needs-work Details | Review
Patch that fixes #699302 bug. (9.02 KB, patch)
2013-05-07 12:19 UTC, Ilya Smelykh
none Details | Review
Patch that fixes #699302 bug. (800 bytes, patch)
2013-05-07 12:46 UTC, Ilya Smelykh
reviewed Details | Review
Patch that fixes #699302 bug. (744 bytes, patch)
2013-05-08 12:04 UTC, Ilya Smelykh
committed Details | Review

Description Ilya Smelykh 2013-04-30 12:31:23 UTC
Looks like problem appears with streams streams without SPS/PPS at the beginning of the stream and if element after parser doesn't accept caps without such information as width and height.

 I've also found Sebastian's comment about possible root case of this bug:
  https://bugzilla.gnome.org/show_bug.cgi?id=698679#c2

Thanks,
Ilya.
Comment 1 Ilya Smelykh 2013-04-30 12:36:48 UTC
Created attachment 242904 [details]
Issue stream

This is issue h264 elementary stream.
Comment 2 Tim-Philipp Müller 2013-05-01 14:26:37 UTC
*** Bug 695206 has been marked as a duplicate of this bug. ***
Comment 3 Sebastian Dröge (slomo) 2013-05-01 15:40:01 UTC
Bug #699398 should fix the same for mpegvideoparse.
Comment 4 Ilya Smelykh 2013-05-06 15:48:17 UTC
Created attachment 243395 [details] [review]
Patch that fixes #699302 bug.

 Please review this patch, I'm not sure that it the correct place in code where  the fix for this bug should be.

Thanks,
Ilya.
Comment 5 Ilya Smelykh 2013-05-06 16:00:14 UTC
Comment on attachment 243395 [details] [review]
Patch that fixes #699302 bug.

>diff --git a/gst/videoparsers/gsth264parse.c b/gst/videoparsers/gsth264parse.c
>index 6da80f2..5b4a50a 100644
>--- a/gst/videoparsers/gsth264parse.c
>+++ b/gst/videoparsers/gsth264parse.c
>@@ -882,7 +882,16 @@ gst_h264_parse_handle_frame (GstBaseParse * parse,
>       }
>     }
> 
>-    gst_h264_parse_process_nal (h264parse, &nalu);
>+    if ((!h264parse->have_sps && nalu.type == GST_H264_NAL_SPS) ||
>+        (!h264parse->have_pps && nalu.type == GST_H264_NAL_PPS) ||
>+        (h264parse->have_sps && h264parse->have_sps)) {
>+      gst_h264_parse_process_nal (h264parse, &nalu);
>+    }
>+    else {
>+      GST_WARNING_OBJECT (h264parse, "no SPS/PPS yet, nal Type: %d, Size: %u will be dropped", nalu.type, nalu.size);
>+      *skipsize = nalu.size;
>+      goto skip;
>+    }
> 
>     if (nonext)
>       break;
Comment 6 Sebastian Dröge (slomo) 2013-05-07 08:35:44 UTC
This is related to bug #646327.
Comment 7 Tim-Philipp Müller 2013-05-07 11:36:02 UTC
Comment on attachment 243395 [details] [review]
Patch that fixes #699302 bug.

>diff --git a/gst/videoparsers/gsth264parse.c b/gst/videoparsers/gsth264parse.c
>index 6da80f2..5b4a50a 100644
>--- a/gst/videoparsers/gsth264parse.c
>+++ b/gst/videoparsers/gsth264parse.c
>@@ -882,7 +882,16 @@ gst_h264_parse_handle_frame (GstBaseParse * parse,
>       }
>     }
> 
>-    gst_h264_parse_process_nal (h264parse, &nalu);
>+    if ((!h264parse->have_sps && nalu.type == GST_H264_NAL_SPS) ||
>+        (!h264parse->have_sps && nalu.type == GST_H264_NAL_PPS) ||
>+        (h264parse->have_sps && h264parse->have_sps)) {
>+      gst_h264_parse_process_nal (h264parse, &nalu);
>+    }

I think there are some have_sps/pps mixups here?
Comment 8 Ilya Smelykh 2013-05-07 12:19:27 UTC
Created attachment 243484 [details] [review]
Patch that fixes #699302 bug.

 There were some typos in last patch so here is the new one.
Comment 9 Ilya Smelykh 2013-05-07 12:46:27 UTC
Created attachment 243487 [details] [review]
Patch that fixes #699302 bug.
Comment 10 Tim-Philipp Müller 2013-05-07 15:24:51 UTC
Comment on attachment 243487 [details] [review]
Patch that fixes #699302 bug.

 
>-    gst_h264_parse_process_nal (h264parse, &nalu);
>+    if ((!h264parse->have_sps && nalu.type == GST_H264_NAL_SPS) ||
>+        (!h264parse->have_pps && nalu.type == GST_H264_NAL_PPS) ||
>+        (h264parse->have_sps && h264parse->have_pps)) {
>+      gst_h264_parse_process_nal (h264parse, &nalu);
>+    }

Apologies for the nitpicking, but shouldn't it be:

if (nalu.type == SPS || nalu.type == PPS ||
   (have_sps && have_pps)) {
  .. process_nal();
}

(there can be multiple PPS, no? And we want to update the SPS even if we already have one (irrespective of whether we have an PPS or not))
Comment 11 Ilya Smelykh 2013-05-08 12:04:52 UTC
Created attachment 243583 [details] [review]
Patch that fixes #699302 bug.

 Patch with fixes which were suggested by Tim.
Comment 12 Sebastian Dröge (slomo) 2013-05-08 12:56:57 UTC
Let's close this bug here and make it a duplicate of #646327. It's exactly the same problem, let's continue there :)

*** This bug has been marked as a duplicate of bug 646327 ***