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 663180 - h264parse tests are not valgrind clean
h264parse tests are not valgrind clean
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal critical
: 0.10.23
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-11-01 17:01 UTC by René Stadler
Modified: 2011-11-18 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description René Stadler 2011-11-01 17:01:03 UTC
make -C tests/check elements/h264parse.valgrind shows a remaining access violation that I don't know how to resolve properly:

==23292== Conditional jump or move depends on uninitialised value(s)
==23292==    at 0x6220305: gst_h264_parse_collect_nal (gsth264parse.c:570)
==23292==    by 0x6220A08: gst_h264_parse_check_valid_frame (gsth264parse.c:689)
==23292==    by 0x665E50D: gst_base_parse_chain (gstbaseparse.c:2381)
==23292==    by 0x62233D7: gst_h264_parse_chain (gsth264parse.c:1413)
[...]

The affected line is:

  complete |= h264parse->picture_start &&
      (nal_type == GST_H264_NAL_SLICE ||
      nal_type == GST_H264_NAL_SLICE_DPA ||
      nal_type == GST_H264_NAL_SLICE_IDR) &&
      /* first_mb_in_slice == 0 considered start of frame */
      (nnalu.data[nnalu.offset + 1] & 0x80);

The violation is in the access to nnalu.data at the specified index. The reason seems to be that gst_h264_parser_identify_nalu() actually returned GST_H264_PARSE_NO_NAL_END (which is not handled...). It's not really clear to me whether the aggregation logic in check_valid_frame is supposed to keep this from happening or collect_nal needs to be more careful. Changing it return FALSE in this case (like for PARSER_ERROR) causes the test to fail, so not sure if this is actually correct and the test is wrong.

Leaving this for the experts to figure out, since I'm not an expert with H264 and the code is too complex for me to infer things...
Comment 1 Mark Nauwelaerts 2011-11-18 16:43:16 UTC
commit 7fae33a661158b061b9466d74cb090c222f3d4d5
Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk>
Date:   Fri Nov 18 12:32:13 2011 +0100

    h264parse: fix up AU collection processing
    
    On the one hand, no need to collect nal if processing last one.
    On the other hand, ensure AU collection processing to have sufficient
    next NAL data in normal cases.
    
    Fixes #663180.