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 769797 - videoparse: format i420, invalid buffer size (regression)
videoparse: format i420, invalid buffer size (regression)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.9.1
Other Linux
: Normal blocker
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 769637
 
 
Reported: 2016-08-12 11:54 UTC by balazs.kreith@gmail.com
Modified: 2016-08-18 06:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Compute plane strides&offsets if no custom ones are set (10.98 KB, patch)
2016-08-15 19:15 UTC, Carlos Rafael Giani
none Details | Review
Compute plane strides&offsets if no custom ones are set v2 (12.49 KB, patch)
2016-08-16 07:33 UTC, Carlos Rafael Giani
committed Details | Review

Description balazs.kreith@gmail.com 2016-08-12 11:54:06 UTC
The following pipeline with gst-plugins-bad 1.8.2 works fine

gst-launch-1.0 filesrc location=foreman_cif.yuv ! videoparse height=288 width=352 framerate=25/1 ! xvimagesink

If I install gst-plugins-bad 1.9.1 then it works not so fine.

ERROR                default video-frame.c:175:gst_video_frame_map_id: invalid buffer size 119040 < 152064
ERROR                default video-frame.c:175:gst_video_frame_map_id: invalid buffer size 119040 < 152064
ERROR                default video-frame.c:175:gst_video_frame_map_id: invalid buffer size 119040 < 152064
ERROR                default video-frame.c:175:gst_video_frame_map_id: invalid buffer size 119040 < 152064

and so on...

The foreman_cif.yuv file you can download from https://media.xiph.org/video/derf/
Comment 1 Sebastian Dröge (slomo) 2016-08-12 13:39:00 UTC
It's also still a problem with the new rawvideoparse in GIT master.
Comment 2 Tim-Philipp Müller 2016-08-12 13:44:53 UTC
Sounds like a regression, marking as blocker.
Comment 3 Sebastian Dröge (slomo) 2016-08-12 13:54:29 UTC
The problem is that the plane offsets and strides are only set initially in gst_raw_video_parse_init_config() and would be updated from caps or property. But they are not updated when e.g. changing the width/height.

gst_raw_video_parse_update_info() is then using the 320x240 I420 values, which gives us the wrong results.
Comment 4 Sebastian Dröge (slomo) 2016-08-12 13:55:59 UTC
gst_raw_video_parse_update_info() also assumes that the last plane is the physically last one. For YV12 that's not necessarily true (V comes before U).
Comment 5 Sebastian Dröge (slomo) 2016-08-12 13:56:31 UTC
Should've been introduced by https://bugzilla.gnome.org/show_bug.cgi?id=760270
Comment 6 Carlos Rafael Giani 2016-08-15 19:15:57 UTC
Created attachment 333376 [details] [review]
Compute plane strides&offsets if no custom ones are set

(In reply to Sebastian Dröge (slomo) from comment #4)
> gst_raw_video_parse_update_info() also assumes that the last plane is the
> physically last one. For YV12 that's not necessarily true (V comes before U).

This patch fixes the reported problems, except this detail. I currently do not know how would I get the physically last plane. Any ideas?
Comment 7 Carlos Rafael Giani 2016-08-15 19:16:58 UTC
Also, once this issue is resolved, we should apply https://bugzilla.gnome.org/show_bug.cgi?id=769637 as well. Its second hunk will need an update due to the new test, but this is an easy fix.
Comment 8 Sebastian Dröge (slomo) 2016-08-16 06:42:45 UTC
(In reply to Carlos Rafael Giani from comment #6)
> Created attachment 333376 [details] [review] [review]
> Compute plane strides&offsets if no custom ones are set
> 
> (In reply to Sebastian Dröge (slomo) from comment #4)
> > gst_raw_video_parse_update_info() also assumes that the last plane is the
> > physically last one. For YV12 that's not necessarily true (V comes before U).
> 
> This patch fixes the reported problems, except this detail. I currently do
> not know how would I get the physically last plane. Any ideas?

The old code in videoparse did that. It iterated over all planes and remembered the physically last one.

Also looking at gst_video_frame_copy() might be useful
Comment 9 Carlos Rafael Giani 2016-08-16 07:33:14 UTC
Created attachment 333393 [details] [review]
Compute plane strides&offsets if no custom ones are set v2

Here is the updated patch with the last plane offset change.
Comment 10 Sebastian Dröge (slomo) 2016-08-18 06:22:53 UTC
commit 91cf5ac69f9c99fe41d60f42b4174915dd135e7b
Author: Carlos Rafael Giani <dv@pseudoterminal.org>
Date:   Tue Aug 16 09:31:40 2016 +0200

    rawvideoparse: Compute plane offsets & strides if no custom ones are set
    
    This is useful to ensure that the offsets and strides are computed if
    only width, height, format etc. in the property config are set.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=769797