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 740673 - codecparsers: some compiler warnings with apple-gcc42
codecparsers: some compiler warnings with apple-gcc42
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Mac OS
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-25 08:11 UTC by Julien Isorce
Modified: 2014-11-25 23:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
codecparsers: fix some compiler warnings (4.14 KB, patch)
2014-11-25 08:14 UTC, Julien Isorce
needs-work Details | Review

Description Julien Isorce 2014-11-25 08:11:18 UTC
With latest gcc from macport (apple-gcc42), it currently fails to build  libcodecparsersbad because of the following warning:

  CC       libgstcodecparsers_1.0_la-gsth265parser.lo
cc1: warnings being treated as errors

gsth265parser.c: In function 'get_default_scaling_lists':
gsth265parser.c:593: warning: comparison is always true due to limited range of data type
gsth265parser.c: In function 'gst_h265_parser_parse_short_term_ref_pic_sets':
gsth265parser.c:768: warning: comparison is always false due to limited range of data type
gsth265parser.c: In function 'gst_h265_parse_vps':
gsth265parser.c:1466: warning: comparison is always false due to limited range of data type
gsth265parser.c:1469: warning: comparison is always false due to limited range of data type
gsth265parser.c:1487: warning: comparison is always false due to limited range of data type
gsth265parser.c:1491: warning: comparison is always false due to limited range of data type
gsth265parser.c: In function 'gst_h265_parser_parse_slice_hdr':
gsth265parser.c:2017: warning: comparison is always false due to limited range of data type
gsth265parser.c:2017: warning: comparison is always false due to limited range of data type
gsth265parser.c:2028: warning: comparison is always false due to limited range of data type
Comment 1 Julien Isorce 2014-11-25 08:14:38 UTC
Created attachment 291436 [details] [review]
codecparsers: fix some compiler warnings

I would like to make sure I do not change the current behavior
Comment 2 Sebastian Dröge (slomo) 2014-11-25 09:29:42 UTC
Review of attachment 291436 [details] [review]:

I think this is not correct

::: gst-libs/gst/codecparsers/gsth265parser.c
@@ -1486,2 @@
     READ_UE_ALLOWED (&nr, vps->num_hrd_parameters, 0, 1024);
-    CHECK_ALLOWED (vps->num_hrd_parameters, 0, 1);

This checks here that the value is 0 or 1... and you remove the check

@@ -1489,3 @@
     if (vps->num_hrd_parameters) {
       READ_UE_ALLOWED (&nr, vps->hrd_layer_set_idx, 0, 1023);
-      CHECK_ALLOWED (vps->hrd_layer_set_idx, 0, 0);

This checks if the value is 0 and only allows 0

@@ -2015,3 @@
       READ_UINT16 (&nr, slice->pic_order_cnt_lsb,
           (sps->log2_max_pic_order_cnt_lsb_minus4 + 4));
-      CHECK_ALLOWED (slice->pic_order_cnt_lsb, 0, G_MAXUINT16);

This one is indeed useless
Comment 3 Julien Isorce 2014-11-25 09:35:38 UTC
The thing is that READ_UE_ALLOWED already calls CHECK_ALLOWED, and uses the input params 0 and 1024 only  with this internal call to CHECK_ALLOWED. So I suspected the next call to CHECK_ALLOWED(0, 1) was wrong.

So it should be either:
READ_UE_ALLOWED (&nr, vps->num_hrd_parameters, 0, 1024);
or
READ_UE_ALLOWED (&nr, vps->num_hrd_parameters, 0, 1);
Comment 4 sreerenj 2014-11-25 09:39:44 UTC
The type of deltaRps should be gint16.
Comment 5 sreerenj 2014-11-25 09:42:51 UTC
(In reply to comment #3)
> The thing is that READ_UE_ALLOWED already calls CHECK_ALLOWED, and uses the
> input params 0 and 1024 only  with this internal call to CHECK_ALLOWED. So I
> suspected the next call to CHECK_ALLOWED(0, 1) was wrong.
> 
> So it should be either:
> READ_UE_ALLOWED (&nr, vps->num_hrd_parameters, 0, 1024);
> or
> READ_UE_ALLOWED (&nr, vps->num_hrd_parameters, 0, 1);

The value of num_hrd_parameters can be in the range of 0 to 1024. But the current specification is only allowing 0 or 1.
Comment 6 Julien Isorce 2014-11-25 09:51:17 UTC
thx for the clarification. 
Then to fix the warning I got on CHECK_ALLOWED (vps->num_hrd_parameters, 0, 1); (warning: comparison is always true due to limited range of
data type), should I create a macro CHECK_ALLOWED_UNSIGNED (vps->num_hrd_parameters, 1)  ?
Comment 7 sreerenj 2014-11-25 10:06:02 UTC
(In reply to comment #6)
> thx for the clarification. 
> Then to fix the warning I got on CHECK_ALLOWED (vps->num_hrd_parameters, 0, 1);
> (warning: comparison is always true due to limited range of
> data type), should I create a macro CHECK_ALLOWED_UNSIGNED
> (vps->num_hrd_parameters, 1)  ?

Why can't you use your new macro CHECK_ALLOWED_MAX(num_hrd_param, 1) after CHECK_ALLOWED() ?
Comment 8 Julien Isorce 2014-11-25 10:08:20 UTC
Ah right, sorry I already forgot I introduced this new macro :)
Comment 9 Julien Isorce 2014-11-25 23:08:37 UTC
commit 3fb3758e7dd1b58e6ae3d8831a220168117f462a
Author: Julien Isorce <j.isorce@samsung.com>
Date:   Sun Nov 23 23:49:50 2014 +0000

    codecparsers: fix some compiler warnings
    
    i686-apple-darwin11-llvm-gcc-4.2 (GCC) 4.2.1
    
    "warning: comparison is always true due to limited
    range of data type"
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=740673