[FFmpeg-devel] libavcodec/videotoolboxenc: Fix for the compiler error for my old mac pro

Submitted by lance.lmwang@gmail.com on June 7, 2019, 9:59 a.m.

Details

Message ID 1559901577-34240-1-git-send-email-lance.lmwang@gmail.com
State New
Headers show

Commit Message

lance.lmwang@gmail.com June 7, 2019, 9:59 a.m.
From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavcodec/videotoolboxenc.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Marvin Scholz June 7, 2019, 12:30 p.m.
On 7 Jun 2019, at 11:59, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>

This looks like a different approach to a similar patch sent before
by Thilo.

> ---
>  libavcodec/videotoolboxenc.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/libavcodec/videotoolboxenc.c 
> b/libavcodec/videotoolboxenc.c
> index 3665581..e4f44e5 100644
> --- a/libavcodec/videotoolboxenc.c
> +++ b/libavcodec/videotoolboxenc.c
> @@ -763,10 +763,14 @@ static int get_cv_pixel_format(AVCodecContext* 
> avctx,
>                                          kCVPixelFormatType_420YpCbCr8PlanarFullRange 
> :
>                                          kCVPixelFormatType_420YpCbCr8Planar;
>      } else if (fmt == AV_PIX_FMT_P010LE) {
> +#ifndef kCVPixelFormatType_420YpCbCr10BiPlanarFullRange
> +        return AVERROR(EINVAL);
> +#else

I do not think this can ever reach the else branch, as
kCVPixelFormatType_420YpCbCr10BiPlanarFullRange is not
a define but a normal symbol and the preprocessor can not
check for symbols using ifdef, just for defines.

>          *av_pixel_format = range == AVCOL_RANGE_JPEG ?
>                                          kCVPixelFormatType_420YpCbCr10BiPlanarFullRange 
> :
>                                          kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
>          *av_pixel_format = 
> kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
> +#endif
>      } else {
>          return AVERROR(EINVAL);
>      }
> -- 
> 2.6.4
>
> _______________________________________________
> 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".
lance.lmwang@gmail.com June 7, 2019, 3:21 p.m.
On Fri, Jun 7, 2019 at 8:36 PM Marvin Scholz <epirat07@gmail.com> wrote:

> On 7 Jun 2019, at 11:59, lance.lmwang@gmail.com wrote:
>
> > From: Limin Wang <lance.lmwang@gmail.com>
> >
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>
> This looks like a different approach to a similar patch sent before
> by Thilo.
>
> ---
> >  libavcodec/videotoolboxenc.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/libavcodec/videotoolboxenc.c
> > b/libavcodec/videotoolboxenc.c
> > index 3665581..e4f44e5 100644
> > --- a/libavcodec/videotoolboxenc.c
> > +++ b/libavcodec/videotoolboxenc.c
> > @@ -763,10 +763,14 @@ static int get_cv_pixel_format(AVCodecContext*
> > avctx,
> >
> kCVPixelFormatType_420YpCbCr8PlanarFullRange
> > :
> >
> kCVPixelFormatType_420YpCbCr8Planar;
> >      } else if (fmt == AV_PIX_FMT_P010LE) {
> > +#ifndef kCVPixelFormatType_420YpCbCr10BiPlanarFullRange
> > +        return AVERROR(EINVAL);
> > +#else
>
> I do not think this can ever reach the else branch, as
> kCVPixelFormatType_420YpCbCr10BiPlanarFullRange is not
> a define but a normal symbol and the preprocessor can not
> check for symbols using ifdef, just for defines.
>
>
Sorry,  I tested with my system successfully and haven't check it's enum
type, now I have updated the patch, please review it.
https://patchwork.ffmpeg.org/patch/13450/




> >          *av_pixel_format = range == AVCOL_RANGE_JPEG ?
> >
> kCVPixelFormatType_420YpCbCr10BiPlanarFullRange
> > :
> >
> kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
> >          *av_pixel_format =
> > kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
> > +#endif
> >      } else {
> >          return AVERROR(EINVAL);
> >      }
> > --
> > 2.6.4
> >
> > _______________________________________________
> > 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".
> _______________________________________________
> 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".
Thilo Borgmann June 7, 2019, 3:52 p.m.
Am 07.06.19 um 17:21 schrieb Lance Wang:
> On Fri, Jun 7, 2019 at 8:36 PM Marvin Scholz <epirat07@gmail.com> wrote:
> 
>> On 7 Jun 2019, at 11:59, lance.lmwang@gmail.com wrote:
>>
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>
>> This looks like a different approach to a similar patch sent before
>> by Thilo.

It is. We'd discussed a possible solution in the corresponding thread:
https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-May/244049.html

I guess I'll implement it next week like said in there - feel free to implement that approach beofre I'll get to it.

-Thilo
lance.lmwang@gmail.com June 7, 2019, 10:16 p.m.
On Fri, Jun 7, 2019 at 11:52 PM Thilo Borgmann <thilo.borgmann@mail.de>
wrote:

> Am 07.06.19 um 17:21 schrieb Lance Wang:
> > On Fri, Jun 7, 2019 at 8:36 PM Marvin Scholz <epirat07@gmail.com> wrote:
> >
> >> On 7 Jun 2019, at 11:59, lance.lmwang@gmail.com wrote:
> >>
> >>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>
> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>
> >> This looks like a different approach to a similar patch sent before
> >> by Thilo.
>
> It is. We'd discussed a possible solution in the corresponding thread:
> https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2019-May/244049.html
>
> I guess I'll implement it next week like said in there - feel free to
> implement that approach beofre I'll get to it.
>
> -Thilo
>
I can't reply to the old thread, please check my updated approach.
https://patchwork.ffmpeg.org/patch/13450/

HAVE_KCVPIXELFORMATTYPE_420YPCBCR10BIPLANARVIDEORANGE is generated in
config.h by configure already.



> _______________________________________________
> 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".

Patch hide | download patch | download mbox

diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c
index 3665581..e4f44e5 100644
--- a/libavcodec/videotoolboxenc.c
+++ b/libavcodec/videotoolboxenc.c
@@ -763,10 +763,14 @@  static int get_cv_pixel_format(AVCodecContext* avctx,
                                         kCVPixelFormatType_420YpCbCr8PlanarFullRange :
                                         kCVPixelFormatType_420YpCbCr8Planar;
     } else if (fmt == AV_PIX_FMT_P010LE) {
+#ifndef kCVPixelFormatType_420YpCbCr10BiPlanarFullRange
+        return AVERROR(EINVAL);
+#else
         *av_pixel_format = range == AVCOL_RANGE_JPEG ?
                                         kCVPixelFormatType_420YpCbCr10BiPlanarFullRange :
                                         kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
         *av_pixel_format = kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange;
+#endif
     } else {
         return AVERROR(EINVAL);
     }