diff mbox

[FFmpeg-devel] lavc/pcm-dvd: Do not use an incompatible pointer on big-endian.

Message ID CAB0OVGr0o60U9LrLLpSzYU4hfDJZDb2AbAiD050D3xoycOG1wA@mail.gmail.com
State Superseded
Headers show

Commit Message

Carl Eugen Hoyos Nov. 1, 2017, 5:33 p.m. UTC
2017-11-01 18:18 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
> On Wed, Nov 1, 2017 at 5:13 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>> Hi!
>>
>> Attached patch silences a gcc warning, tested with Fever.vob
>>
>>
>> @@ -163,10 +162,12 @@ static void *pcm_dvd_decode_samples(AVCodecContext *avctx, const uint8_t *src,
>>     switch (avctx->bits_per_coded_sample) {
>>     case 16: {
>> #if HAVE_BIGENDIAN
>> +        int8_t *dst16 = dst;
>>         bytestream2_get_buffer(&gb, dst16, blocks * s->block_size);
>> -        dst16 += blocks * s->block_size / 2;
>> +        dst16 += blocks * s->block_size;
>> #else
>>         int samples = blocks * avctx->channels;
>> +        int16_t *dst16 = dst;
>>         do {
>>             *dst16++ = bytestream2_get_be16u(&gb);
>>         } while (--samples);
>
> This results in quite misleading code. dst16 is named like that
> because its a 16-bit pointer, using the same pointer with different
> types based on this ifdef seems error-prone in the future.

Agree, new patch attached.

Thank you, Carl Eugen

Comments

Carl Eugen Hoyos Nov. 4, 2017, 7:33 p.m. UTC | #1
2017-11-01 18:33 GMT+01:00 Carl Eugen Hoyos <ceffmpeg@gmail.com>:
> 2017-11-01 18:18 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>> On Wed, Nov 1, 2017 at 5:13 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> Hi!
>>>
>>> Attached patch silences a gcc warning, tested with Fever.vob
>>>
>>>
>>> @@ -163,10 +162,12 @@ static void *pcm_dvd_decode_samples(AVCodecContext *avctx, const uint8_t *src,
>>>     switch (avctx->bits_per_coded_sample) {
>>>     case 16: {
>>> #if HAVE_BIGENDIAN
>>> +        int8_t *dst16 = dst;
>>>         bytestream2_get_buffer(&gb, dst16, blocks * s->block_size);
>>> -        dst16 += blocks * s->block_size / 2;
>>> +        dst16 += blocks * s->block_size;
>>> #else
>>>         int samples = blocks * avctx->channels;
>>> +        int16_t *dst16 = dst;
>>>         do {
>>>             *dst16++ = bytestream2_get_be16u(&gb);
>>>         } while (--samples);
>>
>> This results in quite misleading code. dst16 is named like that
>> because its a 16-bit pointer, using the same pointer with different
>> types based on this ifdef seems error-prone in the future.
>
> Agree, new patch attached.

Ping.

Carl Eugen
Hendrik Leppkes Nov. 6, 2017, 10:53 a.m. UTC | #2
On Wed, Nov 1, 2017 at 6:33 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 2017-11-01 18:18 GMT+01:00 Hendrik Leppkes <h.leppkes@gmail.com>:
>> On Wed, Nov 1, 2017 at 5:13 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> Hi!
>>>
>>> Attached patch silences a gcc warning, tested with Fever.vob
>>>
>>>
>>> @@ -163,10 +162,12 @@ static void *pcm_dvd_decode_samples(AVCodecContext *avctx, const uint8_t *src,
>>>     switch (avctx->bits_per_coded_sample) {
>>>     case 16: {
>>> #if HAVE_BIGENDIAN
>>> +        int8_t *dst16 = dst;
>>>         bytestream2_get_buffer(&gb, dst16, blocks * s->block_size);
>>> -        dst16 += blocks * s->block_size / 2;
>>> +        dst16 += blocks * s->block_size;
>>> #else
>>>         int samples = blocks * avctx->channels;
>>> +        int16_t *dst16 = dst;
>>>         do {
>>>             *dst16++ = bytestream2_get_be16u(&gb);
>>>         } while (--samples);
>>
>> This results in quite misleading code. dst16 is named like that
>> because its a 16-bit pointer, using the same pointer with different
>> types based on this ifdef seems error-prone in the future.
>
> Agree, new patch attached.
>

I think you attached the wrong patch, this is an old version of the
pcm-bluray patch.
diff mbox

Patch

From 8a2b3dd7be86c0494cb2f09088fb1ad92e910571 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos <ceffmpeg@gmail.com>
Date: Wed, 1 Nov 2017 18:24:15 +0100
Subject: [PATCH] lavc/pcm-bluray: Do not use incompatible pointers on
 big-endian.

Fixes the following gcc warnings:
libavcodec/pcm-bluray.c:172:45: warning: passing argument 2 of 'bytestream2_get_buffer' from incompatible pointer type
libavcodec/pcm-bluray.c:192:49: warning: passing argument 2 of 'bytestream2_get_buffer' from incompatible pointer type
---
 libavcodec/pcm-bluray.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavcodec/pcm-bluray.c b/libavcodec/pcm-bluray.c
index 517d7b5..8f961c3 100644
--- a/libavcodec/pcm-bluray.c
+++ b/libavcodec/pcm-bluray.c
@@ -169,7 +169,7 @@  static int pcm_bluray_decode_frame(AVCodecContext *avctx, void *data,
             samples *= num_source_channels;
             if (AV_SAMPLE_FMT_S16 == avctx->sample_fmt) {
 #if HAVE_BIGENDIAN
-                bytestream2_get_buffer(&gb, dst16, buf_size);
+                bytestream2_get_buffer(&gb, frame->data[0], buf_size);
 #else
                 do {
                     *dst16++ = bytestream2_get_be16u(&gb);
@@ -189,8 +189,9 @@  static int pcm_bluray_decode_frame(AVCodecContext *avctx, void *data,
             if (AV_SAMPLE_FMT_S16 == avctx->sample_fmt) {
                 do {
 #if HAVE_BIGENDIAN
-                    bytestream2_get_buffer(&gb, dst16, avctx->channels * 2);
-                    dst16 += avctx->channels;
+                    uint8_t *dst = frame->data[0];
+                    bytestream2_get_buffer(&gb, dst, avctx->channels * 2);
+                    dst += avctx->channels * 2;
 #else
                     channel = avctx->channels;
                     do {
-- 
1.7.10.4