| Message ID | 0499ea09-caa1-994e-1ce6-a15010bd81ad@mail.de |
|---|---|
| State | New |
| Headers | show |
Hi, On Tue, May 14, 2019 at 4:16 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > > $Subject > > Tested compilation only, sanity test actually using it appreciated. > Thanks for the patch. To be completely fair, this is not to fix compilation for specific target systems, but rather to fix compilation on older SDKs (building with newer SDKs you can still build aiming for macOS starting from 10.9, for example). I didn't notice a patch landed on the encoder side that utilized the defines without further checking/ifdefs. Too bad. I think I specifically didn't yet merge the full/limited range patch on the decoder side due to related reasons. I did notice that VLC just re-defined these enum values themselves to stop needing to have ifdefs depending on which SDK is being utilized (https://github.com/videolan/vlc/commit/1b7e1c4bfcda375e2d4e657135aeaf3732e44af2#diff-a11cdb805d111956af60619d7dfa022bR735). I wonder if we should have a helper header that would re-define these enum values with their name. That way the code would look correct, and the resulting binary has the same features independent of the SDK it had been built under. What would be the opinion of people on a solution like this? Best regards, Jan
Am 22.05.19 um 01:41 schrieb Jan Ekström: > Hi, > > On Tue, May 14, 2019 at 4:16 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote: >> >> $Subject >> >> Tested compilation only, sanity test actually using it appreciated. >> > > Thanks for the patch. To be completely fair, this is not to fix > compilation for specific target systems, but rather to fix compilation > on older SDKs (building with newer SDKs you can still build aiming for > macOS starting from 10.9, for example). > > I didn't notice a patch landed on the encoder side that utilized the > defines without further checking/ifdefs. Too bad. I think I > specifically didn't yet merge the full/limited range patch on the > decoder side due to related reasons. > > I did notice that VLC just re-defined these enum values themselves to > stop needing to have ifdefs depending on which SDK is being utilized > (https://github.com/videolan/vlc/commit/1b7e1c4bfcda375e2d4e657135aeaf3732e44af2#diff-a11cdb805d111956af60619d7dfa022bR735). > I wonder if we should have a helper header that would re-define these > enum values with their name. That way the code would look correct, and > the resulting binary has the same features independent of the SDK it > had been built under. > > What would be the opinion of people on a solution like this? Tested with a local definition of the symbols (like a would be header would do). Seems to work for building with -macosx-version-min=XXX. Also checked with VLC, they do these checks via thinks like: #ifndef MAC_OS_X_VERSION_10_13 ... #endif Wich might be a better alternative to what I suggested. Thus I would be fine with a helping header. -Thilo
Am 22.05.19 um 14:27 schrieb Thilo Borgmann: > Am 22.05.19 um 01:41 schrieb Jan Ekström: >> Hi, >> >> On Tue, May 14, 2019 at 4:16 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote: >>> >>> $Subject >>> >>> Tested compilation only, sanity test actually using it appreciated. >>> >> >> Thanks for the patch. To be completely fair, this is not to fix >> compilation for specific target systems, but rather to fix compilation >> on older SDKs (building with newer SDKs you can still build aiming for >> macOS starting from 10.9, for example). >> >> I didn't notice a patch landed on the encoder side that utilized the >> defines without further checking/ifdefs. Too bad. I think I >> specifically didn't yet merge the full/limited range patch on the >> decoder side due to related reasons. >> >> I did notice that VLC just re-defined these enum values themselves to >> stop needing to have ifdefs depending on which SDK is being utilized >> (https://github.com/videolan/vlc/commit/1b7e1c4bfcda375e2d4e657135aeaf3732e44af2#diff-a11cdb805d111956af60619d7dfa022bR735). >> I wonder if we should have a helper header that would re-define these >> enum values with their name. That way the code would look correct, and >> the resulting binary has the same features independent of the SDK it >> had been built under. >> >> What would be the opinion of people on a solution like this? > > Tested with a local definition of the symbols (like a would be header would do). > Seems to work for building with -macosx-version-min=XXX. > > Also checked with VLC, they do these checks via thinks like: > > #ifndef MAC_OS_X_VERSION_10_13 > ... > #endif > > Wich might be a better alternative to what I suggested. > > Thus I would be fine with a helping header. If nobody else cares, should I try to come up with something like this? Or do you want to? -Thilo
On Fri, May 24, 2019 at 6:34 PM Thilo Borgmann <thilo.borgmann@mail.de> wrote: > Am 22.05.19 um 14:27 schrieb Thilo Borgmann: > > Am 22.05.19 um 01:41 schrieb Jan Ekström: > >> Hi, > >> > >> On Tue, May 14, 2019 at 4:16 PM Thilo Borgmann <thilo.borgmann@mail.de> > wrote: > >>> > >>> $Subject > >>> > >>> Tested compilation only, sanity test actually using it appreciated. > >>> > >> > >> Thanks for the patch. To be completely fair, this is not to fix > >> compilation for specific target systems, but rather to fix compilation > >> on older SDKs (building with newer SDKs you can still build aiming for > >> macOS starting from 10.9, for example). > >> > >> I didn't notice a patch landed on the encoder side that utilized the > >> defines without further checking/ifdefs. Too bad. I think I > >> specifically didn't yet merge the full/limited range patch on the > >> decoder side due to related reasons. > >> > >> I did notice that VLC just re-defined these enum values themselves to > >> stop needing to have ifdefs depending on which SDK is being utilized > >> ( > https://github.com/videolan/vlc/commit/1b7e1c4bfcda375e2d4e657135aeaf3732e44af2#diff-a11cdb805d111956af60619d7dfa022bR735 > ). > >> I wonder if we should have a helper header that would re-define these > >> enum values with their name. That way the code would look correct, and > >> the resulting binary has the same features independent of the SDK it > >> had been built under. > >> > >> What would be the opinion of people on a solution like this? > > > > Tested with a local definition of the symbols (like a would be header > would do). > > Seems to work for building with -macosx-version-min=XXX. > > > > Also checked with VLC, they do these checks via thinks like: > > > > #ifndef MAC_OS_X_VERSION_10_13 > > ... > > #endif > > > > Wich might be a better alternative to what I suggested. > > > > Thus I would be fine with a helping header. > > If nobody else cares, should I try to come up with something like this? Or > do you want to? > If you're willing to maintain the header as new values are added, it will make it easier to read the code. > > -Thilo > _______________________________________________ > 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".
diff --git a/libavcodec/videotoolboxenc.c b/libavcodec/videotoolboxenc.c index 3665581..24e0b62 100644 --- a/libavcodec/videotoolboxenc.c +++ b/libavcodec/videotoolboxenc.c @@ -762,12 +762,17 @@ static int get_cv_pixel_format(AVCodecContext* avctx, *av_pixel_format = range == AVCOL_RANGE_JPEG ? kCVPixelFormatType_420YpCbCr8PlanarFullRange : kCVPixelFormatType_420YpCbCr8Planar; - } else if (fmt == AV_PIX_FMT_P010LE) { + } +#if ((!TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300) || \ + (TARGET_OS_IPHONE && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000)) + else if (fmt == AV_PIX_FMT_P010LE) { *av_pixel_format = range == AVCOL_RANGE_JPEG ? kCVPixelFormatType_420YpCbCr10BiPlanarFullRange : kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange; *av_pixel_format = kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange; - } else { + } +#endif + else { return AVERROR(EINVAL); } @@ -1991,6 +1996,8 @@ static int get_cv_pixel_info( strides[2] = frame ? frame->linesize[2] : (avctx->width + 1) / 2; break; +#if ((!TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300) || \ + (TARGET_OS_IPHONE && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000)) case AV_PIX_FMT_P010LE: *plane_count = 2; widths[0] = avctx->width; @@ -2001,6 +2008,7 @@ static int get_cv_pixel_info( heights[1] = (avctx->height + 1) / 2; strides[1] = frame ? frame->linesize[1] : ((avctx->width + 1) / 2 + 63) & -64; break; +#endif default: av_log( @@ -2488,7 +2496,10 @@ static const enum AVPixelFormat hevc_pix_fmts[] = { AV_PIX_FMT_VIDEOTOOLBOX, AV_PIX_FMT_NV12, AV_PIX_FMT_YUV420P, +#if ((!TARGET_OS_IPHONE && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300) || \ + (TARGET_OS_IPHONE && __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000)) AV_PIX_FMT_P010LE, +#endif AV_PIX_FMT_NONE };
$Subject Tested compilation only, sanity test actually using it appreciated. Thanks, Thilo From 104b26ca0eab116dcd49b5f2090067b76b5bfc70 Mon Sep 17 00:00:00 2001 From: Thilo Borgmann <thilo.borgmann@mail.de> Date: Tue, 14 May 2019 15:11:38 +0200 Subject: [PATCH] lavc/videotoolboxenc: Fix compilation for IOS < 11.0 and OSX < 10.13. Prior OS versions are missing the 10bpp formats. --- libavcodec/videotoolboxenc.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)