Re: [PATCH] Fix a parsing error in dr_02.c

On Mon, Jun 1, 2015 at 9:48 AM, Jean-Paul Saman <[email protected]>
wrote:

> Daniel,
>
> On Sun, May 31, 2015 at 10:03 PM, Daniel Kamil Kozar <[email protected]>
> wrote:
>
>> b_mpeg2 in dvbpsi_vstream_dr_t corresponds to what the H.222.0 refers to
>> as the "MPEG_1_only_flag". If the flag is not set, it means that no
>> extra data is contained at the end of the descriptor. The code in
>> dr_02.c did exactly the opposite.
>> ---
>>  src/descriptors/dr_02.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/descriptors/dr_02.c b/src/descriptors/dr_02.c
>> index 774a1c0..1c35d83 100644
>> --- a/src/descriptors/dr_02.c
>> +++ b/src/descriptors/dr_02.c
>> @@ -60,7 +60,7 @@ dvbpsi_vstream_dr_t *
>> dvbpsi_DecodeVStreamDr(dvbpsi_descriptor_t * p_descriptor)
>>    if(!p_decoded) return NULL;
>>
>>    /* Decode data and check the length */
>> -  p_decoded->b_mpeg2 = !((p_descriptor->p_data[0] & 0x04) ? true :
>> false);
>> +  p_decoded->b_mpeg2 = (p_descriptor->p_data[0] & 0x04) ? true : false;
>>    if(    (!p_decoded->b_mpeg2 && (p_descriptor->i_length != 1))
>>        || (p_decoded->b_mpeg2 && (p_descriptor->i_length != 3)))
>>    {
>> --
>> 2.4.2
>>
>>
> Could you double check the specs, because it would mean a revert of
> 25cdb9fd, which I believe is correct in the mentioned specs.
> That H.222.0 and " ISO/IEC 13818-1 : 2000, table 2-40 and 2.6.3"
> contradict each other is quite unlikely IMHO.
>
> $> git show 25cdb9fd
> commit 25cdb9fd7ce804ec69b91fa1b0887435c474b32d
> Author: Jean-Baptiste Kempf <[email protected]>
> Date:   Tue Aug 12 04:04:47 2014 +0200
>
>     Correctly flag mpeg2 video data in video_stream_descriptor
>
>     See ISO/IEC 13818-1 : 2000, table 2-40 and 2.6.3
>
>     See https://trac.videolan.org/vlc/ticket/4200
>
>     Signed-off-by: Jean-Paul Saman <[email protected]>
>
> diff --git a/src/descriptors/dr_02.c b/src/descriptors/dr_02.c
> index 1c35d83..774a1c0 100644
> --- a/src/descriptors/dr_02.c
> +++ b/src/descriptors/dr_02.c
> @@ -60,7 +60,7 @@ dvbpsi_vstream_dr_t *
> dvbpsi_DecodeVStreamDr(dvbpsi_descriptor_t * p_descriptor)
>    if(!p_decoded) return NULL;
>
>    /* Decode data and check the length */
> -  p_decoded->b_mpeg2 = (p_descriptor->p_data[0] & 0x04) ? true : false;
> +  p_decoded->b_mpeg2 = !((p_descriptor->p_data[0] & 0x04) ? true : false);
>    if(    (!p_decoded->b_mpeg2 && (p_descriptor->i_length != 1))
>        || (p_decoded->b_mpeg2 && (p_descriptor->i_length != 3)))
>    {
>

H.222.0 page 65 says:

"MPEG_1_only_flag – This is a 1-bit field which when set to '1' indicates
that the video stream contains only
ISO/IEC 11172-2 data. If set to '0' the video stream may contain both ITU-T
Rec. H.262 | ISO/IEC 13818-2 video data
and constrained parameter ISO/IEC 11172-2 video data."

Meaning that if this flag is '0', then there is extra data. If we now
translate that to b_mpeg2 boolean in libdvbpsi, then it means that
b_mpeg2 should be the inverse of the MPEG_1_only_flag, or b_mpeg2 =
!(MPEG_1_only_flag).

On bit level that becomes:
(p_data[0] & 0x04) => 0 when the flag is not set, thus extra data and
variable p_decoded->b_mpeg2 should become true
(p_data[0] & 0x04) => 1 when the flag is set, thus mpeg1 only data and
variable p_decoded->b_mpeg2 should become false

Hence I believe the current code is correct, since that is exactly what
happens now.


> Kind regards,
>
> Jean-Paul Saman
>
>> _______________________________________________
>> libdvbpsi-devel mailing list
>> [email protected]
>> https://mailman.videolan.org/listinfo/libdvbpsi-devel
>>
>
>

_______________________________________________
libdvbpsi-devel mailing list
[email protected]
https://mailman.videolan.org/listinfo/libdvbpsi-devel