diff mbox

[FFmpeg-devel,1/2] libavcodec/zmbv: change 24-bit decoder channel order, from RGB24 to BGR24

Message ID CAHMcQqY=2VpQU+s_FPqPfU_BmkHWN0-ohAuDLd5rWVE_MBbsVA@mail.gmail.com
State New
Headers show

Commit Message

matthew.w.fearnley@gmail.com March 29, 2019, 10:23 p.m. UTC 
On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.se> wrote:

> tis 2019-03-26 klockan 22:13 +0000 skrev Matthew Fearnley:
> > This brings the channel order in line with that used in 32-bit mode
> (BGR0).
> >
> > 24-bit decoding is disabled by default (#ifdef ZMBV_ENABLE_24BPP), and no
> > prior encoders or sample videos are known to exist for this bit depth, so
> > I consider this change in implementation is unlikely to affect anyone.
> >
> > The decision has been made in agreement with the DOSBox Development Team
> > > (dosbox.crew@gmail.com), specifically with harekiet, who wrote the
> original
> > codec.
>
> I can confirm this
>
> > Additional minor fix: use PTRDIFF_SPECIFIER for `src - c->decomp_buf`.
> > Other bit depths saw this change in ced0d6c14d, but this instance was
> > missed, presumably because of the #ifdef block.
>
> I think it'd be best to split this off into its own patch, even if it's
> trivial
>
Yeah, I think you're right.
I'm attaching two patches here, if that works..

>
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Comments

Tomas Härdin April 1, 2019, 9:28 a.m. UTC | #1 
fre 2019-03-29 klockan 22:23 +0000 skrev Matthew Fearnley:
> > On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > > Additional minor fix: use PTRDIFF_SPECIFIER for `src - c->decomp_buf`.
> > > Other bit depths saw this change in ced0d6c14d, but this instance was
> > > missed, presumably because of the #ifdef block.
> > 
> > I think it'd be best to split this off into its own patch, even if it's
> > trivial
> > 
> 
> Yeah, I think you're right.
> I'm attaching two patches here, if that works..

You got the commit messages mixed up :) Otherwise they look OK

/Tomas
matthew.w.fearnley@gmail.com April 1, 2019, 12:29 p.m. UTC | #2 
> On 1 Apr 2019, at 10:28, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> 
> fre 2019-03-29 klockan 22:23 +0000 skrev Matthew Fearnley:
>>>> On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.se> wrote:
>>>> Additional minor fix: use PTRDIFF_SPECIFIER for `src - c->decomp_buf`.
>>>> Other bit depths saw this change in ced0d6c14d, but this instance was
>>>> missed, presumably because of the #ifdef block.
>>> 
>>> I think it'd be best to split this off into its own patch, even if it's
>>> trivial
>>> 
>> 
>> Yeah, I think you're right.
>> I'm attaching two patches here, if that works..
> 
> You got the commit messages mixed up :) Otherwise they look OK
Doh!
Do I need to correct that, or is it easy enough for you to fix up manually?
> 
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Tomas Härdin April 1, 2019, 12:39 p.m. UTC | #3 
mån 2019-04-01 klockan 13:29 +0100 skrev Matthew Fearnley:
> > On 1 Apr 2019, at 10:28, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > 
> > fre 2019-03-29 klockan 22:23 +0000 skrev Matthew Fearnley:
> > > > > On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.s
> > > > > e> wrote:
> > > > > Additional minor fix: use PTRDIFF_SPECIFIER for `src - c-
> > > > > >decomp_buf`.
> > > > > Other bit depths saw this change in ced0d6c14d, but this
> > > > > instance was
> > > > > missed, presumably because of the #ifdef block.
> > > > 
> > > > I think it'd be best to split this off into its own patch, even
> > > > if it's
> > > > trivial
> > > > 
> > > 
> > > Yeah, I think you're right.
> > > I'm attaching two patches here, if that works..
> > 
> > You got the commit messages mixed up :) Otherwise they look OK
> 
> Doh!
> Do I need to correct that, or is it easy enough for you to fix up
> manually?

Easy enough to fix. I'll wait a day for more comments before pushing

/Tomas
Tomas Härdin April 2, 2019, 3:48 p.m. UTC | #4 
mån 2019-04-01 klockan 13:29 +0100 skrev Matthew Fearnley:
> > > > On 1 Apr 2019, at 10:28, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > 
> > fre 2019-03-29 klockan 22:23 +0000 skrev Matthew Fearnley:
> > > > > On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> > > > > Additional minor fix: use PTRDIFF_SPECIFIER for `src - c->decomp_buf`.
> > > > > Other bit depths saw this change in ced0d6c14d, but this instance was
> > > > > missed, presumably because of the #ifdef block.
> > > > 
> > > > I think it'd be best to split this off into its own patch, even if it's
> > > > trivial
> > > > 
> > > 
> > > Yeah, I think you're right.
> > > I'm attaching two patches here, if that works..
> > 
> > You got the commit messages mixed up :) Otherwise they look OK
> 
> Doh!
> Do I need to correct that, or is it easy enough for you to fix up manually?

Fixed and pushed.

/Tomas
matthew.w.fearnley@gmail.com April 2, 2019, 8:16 p.m. UTC | #5 
> On 2 Apr 2019, at 16:48, Tomas Härdin <tjoppen@acc.umu.se> wrote:
> 
> mån 2019-04-01 klockan 13:29 +0100 skrev Matthew Fearnley:
>>>>> On 1 Apr 2019, at 10:28, Tomas Härdin <tjoppen@acc.umu.se> wrote:
>>> 
>>> fre 2019-03-29 klockan 22:23 +0000 skrev Matthew Fearnley:
>>>>>> On Wed, 27 Mar 2019 at 09:42, Tomas Härdin <tjoppen@acc.umu.se> wrote:
>>>>>> Additional minor fix: use PTRDIFF_SPECIFIER for `src - c->decomp_buf`.
>>>>>> Other bit depths saw this change in ced0d6c14d, but this instance was
>>>>>> missed, presumably because of the #ifdef block.
>>>>> 
>>>>> I think it'd be best to split this off into its own patch, even if it's
>>>>> trivial
>>>>> 
>>>> 
>>>> Yeah, I think you're right.
>>>> I'm attaching two patches here, if that works..
>>> 
>>> You got the commit messages mixed up :) Otherwise they look OK
>> 
>> Doh!
>> Do I need to correct that, or is it easy enough for you to fix up manually?
> 
> Fixed and pushed.
Thanks so much.
By the way, is there a recommended way to enable #ifdef blocks when building ffmpeg? I’ve been adding #defines of the top of the source file, which does the job, but it feels like a bit of a kludge from a source-control perspective.
> 
> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Carl Eugen Hoyos April 2, 2019, 9:49 p.m. UTC | #6 
2019-04-02 22:16 GMT+02:00, Matthew Fearnley <matthew.w.fearnley@gmail.com>:

> By the way, is there a recommended way to enable #ifdef blocks
> when building ffmpeg?

--extra-cflags come to mind.

Carl Eugen
diff mbox

Patch

From 905eb8b403281f180148bef50804af740d411b54 Mon Sep 17 00:00:00 2001
From: Matthew Fearnley <matthew.w.fearnley@gmail.com>
Date: Fri, 29 Mar 2019 22:03:03 +0000
Subject: [PATCH 2/3] libavcodec/zmbv: use PTRDIFF_SPECIFIER for `src -
 c->decomp_buf`.

Other bit depths saw this change in ced0d6c14d, but this instance was
presumably missed because of the #ifdef block.
---
 libavcodec/zmbv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/zmbv.c b/libavcodec/zmbv.c
index 71ec2cd424..898b62d065 100644
--- a/libavcodec/zmbv.c
+++ b/libavcodec/zmbv.c
@@ -473,7 +473,7 @@  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPac
             c->bpp = 24;
             decode_intra = zmbv_decode_intra;
             c->decode_xor = zmbv_decode_xor_24;
-            avctx->pix_fmt = AV_PIX_FMT_RGB24;
+            avctx->pix_fmt = AV_PIX_FMT_BGR24;
             c->stride = c->width * 3;
             break;
 #endif //ZMBV_ENABLE_24BPP
-- 
2.17.1