diff mbox series

[FFmpeg-devel] ffmpeg: remove ffmpeg_videotoolbox

Message ID 20211113214129.51160-1-rcombs@rcombs.me
State New
Headers show
Series [FFmpeg-devel] ffmpeg: remove ffmpeg_videotoolbox | expand

Checks

Context Check Description
andriy/configurex86 warning Failed to apply patch
andriy/configureppc warning Failed to apply patch

Commit Message

rcombs Nov. 13, 2021, 9:41 p.m. UTC
This was almost completely redundant. The only functionality that's no longer
available after this removal is the videotoolbox_pixfmt arg, which has been
obsolete for several years.
---
 fftools/Makefile              |   4 -
 fftools/ffmpeg.c              |  26 -----
 fftools/ffmpeg.h              |  10 --
 fftools/ffmpeg_opt.c          |  28 +----
 fftools/ffmpeg_videotoolbox.c | 200 ----------------------------------
 5 files changed, 4 insertions(+), 264 deletions(-)
 delete mode 100644 fftools/ffmpeg_videotoolbox.c

Comments

Michael Niedermayer Nov. 14, 2021, 2:31 p.m. UTC | #1
On Sat, Nov 13, 2021 at 03:41:29PM -0600, rcombs wrote:
> This was almost completely redundant. The only functionality that's no longer
> available after this removal is the videotoolbox_pixfmt arg, which has been
> obsolete for several years.
> ---
>  fftools/Makefile              |   4 -
>  fftools/ffmpeg.c              |  26 -----
>  fftools/ffmpeg.h              |  10 --
>  fftools/ffmpeg_opt.c          |  28 +----
>  fftools/ffmpeg_videotoolbox.c | 200 ----------------------------------
>  5 files changed, 4 insertions(+), 264 deletions(-)
>  delete mode 100644 fftools/ffmpeg_videotoolbox.c

This breaks fate, ive not look at why

make fate-sp5x
TEST    sp5x
--- ./tests/ref/fate/sp5x	2021-11-01 13:29:46.875005546 +0100
+++ tests/data/fate/sp5x	2021-11-14 15:29:41.747477728 +0100
@@ -9,22 +9,12 @@
 #sample_rate 1: 8000
 #channel_layout 1: 4
 #channel_layout_name 1: mono
-0,          0,          0,        1,   115200, 0x8ebcb7f8
 1,          0,          0,     1024,     2048, 0x366ee71c
-0,          1,          1,        1,   115200, 0x1fa8e673
 1,       1024,       1024,     1024,     2048, 0xc62f0414
-0,          2,          2,        1,   115200, 0xec07fb6a
 1,       2048,       2048,     1024,     2048, 0x754e0f19
-0,          3,          3,        1,   115200, 0x6773a8c3
 1,       3072,       3072,     1024,     2048, 0x4a44152a
-0,          4,          4,        1,   115200, 0x0d279643
 1,       4096,       4096,     1024,     2048, 0x4fd3ff01
-0,          5,          5,        1,   115200, 0xb33796e4
-0,          6,          6,        1,   115200, 0xfe11fc79
 1,       5120,       5120,     1024,     2048, 0x11c3fa1b
-0,          7,          7,        1,   115200, 0x4ac8e31b
 1,       6144,       6144,     1024,     2048, 0x9945fa06
-0,          8,          8,        1,   115200, 0x15317942
 1,       7168,       7168,     1024,     2048, 0x12e5071a
-0,          9,          9,        1,   115200, 0x07803f0e
 1,       8192,       8192,       22,       44, 0x7ad110e8
Test sp5x failed. Look at tests/data/fate/sp5x.err for details.
tests/Makefile:257: recipe for target 'fate-sp5x' failed
make: *** [fate-sp5x] Error 1


[...]
James Almer Nov. 14, 2021, 2:51 p.m. UTC | #2
On 11/13/2021 6:41 PM, rcombs wrote:
> This was almost completely redundant. The only functionality that's no longer
> available after this removal is the videotoolbox_pixfmt arg, which has been
> obsolete for several years.
> ---
>   fftools/Makefile              |   4 -
>   fftools/ffmpeg.c              |  26 -----
>   fftools/ffmpeg.h              |  10 --
>   fftools/ffmpeg_opt.c          |  28 +----
>   fftools/ffmpeg_videotoolbox.c | 200 ----------------------------------
>   5 files changed, 4 insertions(+), 264 deletions(-)
>   delete mode 100644 fftools/ffmpeg_videotoolbox.c
> 
> diff --git a/fftools/Makefile b/fftools/Makefile
> index 5234932ab0..da420786eb 100644
> --- a/fftools/Makefile
> +++ b/fftools/Makefile
> @@ -10,10 +10,6 @@ ALLAVPROGS   = $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
>   ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
>   
>   OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
> -ifndef CONFIG_VIDEOTOOLBOX
> -OBJS-ffmpeg-$(CONFIG_VDA)          += fftools/ffmpeg_videotoolbox.o
> -endif
> -OBJS-ffmpeg-$(CONFIG_VIDEOTOOLBOX) += fftools/ffmpeg_videotoolbox.o
>   
>   define DOFFTOOL
>   OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 9d4f9d7a2b..358256e589 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -2896,32 +2896,6 @@ static enum AVPixelFormat get_format(AVCodecContext *s, const enum AVPixelFormat
>                   }
>                   continue;
>               }
> -        } else {

You need to keep this else to do a continue; in order to not break the 
loop, but while at it you could change the if (config) check above with 
if (!config) continue; then reindent the rest of the code.

> -            const HWAccel *hwaccel = NULL;
> -            int i;
> -            for (i = 0; hwaccels[i].name; i++) {
> -                if (hwaccels[i].pix_fmt == *p) {
> -                    hwaccel = &hwaccels[i];
> -                    break;
> -                }
> -            }
> -            if (!hwaccel) {
> -                // No hwaccel supporting this pixfmt.
> -                continue;
> -            }
> -            if (hwaccel->id != ist->hwaccel_id) {
> -                // Does not match requested hwaccel.
> -                continue;
> -            }
> -
> -            ret = hwaccel->init(s);
> -            if (ret < 0) {
> -                av_log(NULL, AV_LOG_FATAL,
> -                       "%s hwaccel requested for input stream #%d:%d, "
> -                       "but cannot be initialized.\n", hwaccel->name,
> -                       ist->file_index, ist->st->index);
> -                return AV_PIX_FMT_NONE;
> -            }
>           }
>   
>           if (ist->hw_frames_ctx) {
diff mbox series

Patch

diff --git a/fftools/Makefile b/fftools/Makefile
index 5234932ab0..da420786eb 100644
--- a/fftools/Makefile
+++ b/fftools/Makefile
@@ -10,10 +10,6 @@  ALLAVPROGS   = $(AVBASENAMES:%=%$(PROGSSUF)$(EXESUF))
 ALLAVPROGS_G = $(AVBASENAMES:%=%$(PROGSSUF)_g$(EXESUF))
 
 OBJS-ffmpeg                        += fftools/ffmpeg_opt.o fftools/ffmpeg_filter.o fftools/ffmpeg_hw.o
-ifndef CONFIG_VIDEOTOOLBOX
-OBJS-ffmpeg-$(CONFIG_VDA)          += fftools/ffmpeg_videotoolbox.o
-endif
-OBJS-ffmpeg-$(CONFIG_VIDEOTOOLBOX) += fftools/ffmpeg_videotoolbox.o
 
 define DOFFTOOL
 OBJS-$(1) += fftools/cmdutils.o fftools/$(1).o $(OBJS-$(1)-yes)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 9d4f9d7a2b..358256e589 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -2896,32 +2896,6 @@  static enum AVPixelFormat get_format(AVCodecContext *s, const enum AVPixelFormat
                 }
                 continue;
             }
-        } else {
-            const HWAccel *hwaccel = NULL;
-            int i;
-            for (i = 0; hwaccels[i].name; i++) {
-                if (hwaccels[i].pix_fmt == *p) {
-                    hwaccel = &hwaccels[i];
-                    break;
-                }
-            }
-            if (!hwaccel) {
-                // No hwaccel supporting this pixfmt.
-                continue;
-            }
-            if (hwaccel->id != ist->hwaccel_id) {
-                // Does not match requested hwaccel.
-                continue;
-            }
-
-            ret = hwaccel->init(s);
-            if (ret < 0) {
-                av_log(NULL, AV_LOG_FATAL,
-                       "%s hwaccel requested for input stream #%d:%d, "
-                       "but cannot be initialized.\n", hwaccel->name,
-                       ist->file_index, ist->st->index);
-                return AV_PIX_FMT_NONE;
-            }
         }
 
         if (ist->hw_frames_ctx) {
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index 30225e9ffe..e67e84fada 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -60,16 +60,8 @@  enum HWAccelID {
     HWACCEL_NONE = 0,
     HWACCEL_AUTO,
     HWACCEL_GENERIC,
-    HWACCEL_VIDEOTOOLBOX,
 };
 
-typedef struct HWAccel {
-    const char *name;
-    int (*init)(AVCodecContext *s);
-    enum HWAccelID id;
-    enum AVPixelFormat pix_fmt;
-} HWAccel;
-
 typedef struct HWDevice {
     const char *name;
     enum AVHWDeviceType type;
@@ -628,7 +620,6 @@  extern int stdin_interaction;
 extern int frame_bits_per_raw_sample;
 extern AVIOContext *progress_avio;
 extern float max_error_rate;
-extern char *videotoolbox_pixfmt;
 
 extern char *filter_nbthreads;
 extern int filter_complex_nbthreads;
@@ -638,7 +629,6 @@  extern int auto_conversion_filters;
 extern const AVIOInterruptCB int_cb;
 
 extern const OptionDef options[];
-extern const HWAccel hwaccels[];
 #if CONFIG_QSV
 extern char *qsv_device;
 #endif
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index ab4c63a362..65ac855b6c 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -134,12 +134,6 @@  static const char *const opt_name_enc_time_bases[]            = {"enc_time_base"
     }\
 }
 
-const HWAccel hwaccels[] = {
-#if CONFIG_VIDEOTOOLBOX
-    { "videotoolbox", videotoolbox_init, HWACCEL_VIDEOTOOLBOX, AV_PIX_FMT_VIDEOTOOLBOX },
-#endif
-    { 0 },
-};
 HWDevice *filter_hw_device;
 
 char *vstats_filename;
@@ -950,21 +944,10 @@  static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
                 else if (!strcmp(hwaccel, "auto"))
                     ist->hwaccel_id = HWACCEL_AUTO;
                 else {
-                    enum AVHWDeviceType type;
-                    int i;
-                    for (i = 0; hwaccels[i].name; i++) {
-                        if (!strcmp(hwaccels[i].name, hwaccel)) {
-                            ist->hwaccel_id = hwaccels[i].id;
-                            break;
-                        }
-                    }
-
-                    if (!ist->hwaccel_id) {
-                        type = av_hwdevice_find_type_by_name(hwaccel);
-                        if (type != AV_HWDEVICE_TYPE_NONE) {
-                            ist->hwaccel_id = HWACCEL_GENERIC;
-                            ist->hwaccel_device_type = type;
-                        }
+                    enum AVHWDeviceType type = av_hwdevice_find_type_by_name(hwaccel);
+                    if (type != AV_HWDEVICE_TYPE_NONE) {
+                        ist->hwaccel_id = HWACCEL_GENERIC;
+                        ist->hwaccel_device_type = type;
                     }
 
                     if (!ist->hwaccel_id) {
@@ -3763,9 +3746,6 @@  const OptionDef options[] = {
     { "hwaccel_output_format", OPT_VIDEO | OPT_STRING | HAS_ARG | OPT_EXPERT |
                           OPT_SPEC | OPT_INPUT,                                  { .off = OFFSET(hwaccel_output_formats) },
         "select output format used with HW accelerated decoding", "format" },
-#if CONFIG_VIDEOTOOLBOX
-    { "videotoolbox_pixfmt", HAS_ARG | OPT_STRING | OPT_EXPERT, { &videotoolbox_pixfmt}, "" },
-#endif
     { "hwaccels",         OPT_EXIT,                                              { .func_arg = show_hwaccels },
         "show available HW acceleration methods" },
     { "autorotate",       HAS_ARG | OPT_BOOL | OPT_SPEC |
diff --git a/fftools/ffmpeg_videotoolbox.c b/fftools/ffmpeg_videotoolbox.c
deleted file mode 100644
index 66eaa572c8..0000000000
--- a/fftools/ffmpeg_videotoolbox.c
+++ /dev/null
@@ -1,200 +0,0 @@ 
-/*
- * This file is part of FFmpeg.
- *
- * FFmpeg is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * FFmpeg is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with FFmpeg; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include "config.h"
-
-#if HAVE_UTGETOSTYPEFROMSTRING
-#include <CoreServices/CoreServices.h>
-#endif
-
-#include "libavcodec/avcodec.h"
-#include "libavcodec/videotoolbox.h"
-#include "libavutil/imgutils.h"
-#include "ffmpeg.h"
-
-typedef struct VTContext {
-    AVFrame *tmp_frame;
-    int log_once;
-} VTContext;
-
-char *videotoolbox_pixfmt;
-
-static int videotoolbox_retrieve_data(AVCodecContext *s, AVFrame *frame)
-{
-    InputStream *ist = s->opaque;
-    VTContext  *vt = ist->hwaccel_ctx;
-    CVPixelBufferRef pixbuf = (CVPixelBufferRef)frame->data[3];
-    OSType pixel_format = CVPixelBufferGetPixelFormatType(pixbuf);
-    CVReturn err;
-    uint8_t *data[4] = { 0 };
-    int linesize[4] = { 0 };
-    int planes, ret, i;
-
-    if (frame->format == ist->hwaccel_output_format) {
-        av_log_once(s, AV_LOG_INFO, AV_LOG_TRACE, &vt->log_once,
-            "There is no video filter for videotoolbox pix_fmt now, remove the "
-            "-hwaccel_output_format option if video filter doesn't work\n");
-        return 0;
-    }
-
-    av_frame_unref(vt->tmp_frame);
-
-    switch (pixel_format) {
-    case kCVPixelFormatType_420YpCbCr8Planar: vt->tmp_frame->format = AV_PIX_FMT_YUV420P; break;
-    case kCVPixelFormatType_422YpCbCr8:       vt->tmp_frame->format = AV_PIX_FMT_UYVY422; break;
-    case kCVPixelFormatType_32BGRA:           vt->tmp_frame->format = AV_PIX_FMT_BGRA; break;
-#ifdef kCFCoreFoundationVersionNumber10_7
-    case kCVPixelFormatType_420YpCbCr8BiPlanarVideoRange:
-    case kCVPixelFormatType_420YpCbCr8BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_NV12; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_420YPCBCR10BIPLANARVIDEORANGE
-    case kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange:
-    case kCVPixelFormatType_420YpCbCr10BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_P010; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_422YPCBCR8BIPLANARVIDEORANGE
-    case kCVPixelFormatType_422YpCbCr8BiPlanarVideoRange:
-    case kCVPixelFormatType_422YpCbCr8BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_NV16; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_422YPCBCR10BIPLANARVIDEORANGE
-    case kCVPixelFormatType_422YpCbCr10BiPlanarVideoRange:
-    case kCVPixelFormatType_422YpCbCr10BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_NV20; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_422YPCBCR16BIPLANARVIDEORANGE
-    case kCVPixelFormatType_422YpCbCr16BiPlanarVideoRange: vt->tmp_frame->format = AV_PIX_FMT_P216; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_444YPCBCR8BIPLANARVIDEORANGE
-    case kCVPixelFormatType_444YpCbCr8BiPlanarVideoRange:
-    case kCVPixelFormatType_444YpCbCr8BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_NV24; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_444YPCBCR10BIPLANARVIDEORANGE
-    case kCVPixelFormatType_444YpCbCr10BiPlanarVideoRange:
-    case kCVPixelFormatType_444YpCbCr10BiPlanarFullRange: vt->tmp_frame->format = AV_PIX_FMT_P410; break;
-#endif
-#if HAVE_KCVPIXELFORMATTYPE_444YPCBCR16BIPLANARVIDEORANGE
-    case kCVPixelFormatType_444YpCbCr16BiPlanarVideoRange: vt->tmp_frame->format = AV_PIX_FMT_P416; break;
-#endif
-    case kCVPixelFormatType_4444AYpCbCr16: vt->tmp_frame->format = AV_PIX_FMT_AYUV64; break;
-    default:
-        av_log(NULL, AV_LOG_ERROR,
-               "%s: Unsupported pixel format: %s\n",
-               av_fourcc2str(s->codec_tag), videotoolbox_pixfmt);
-        return AVERROR(ENOSYS);
-    }
-
-    vt->tmp_frame->width  = frame->width;
-    vt->tmp_frame->height = frame->height;
-    ret = av_frame_get_buffer(vt->tmp_frame, 0);
-    if (ret < 0)
-        return ret;
-
-    err = CVPixelBufferLockBaseAddress(pixbuf, kCVPixelBufferLock_ReadOnly);
-    if (err != kCVReturnSuccess) {
-        av_log(NULL, AV_LOG_ERROR, "Error locking the pixel buffer.\n");
-        return AVERROR_UNKNOWN;
-    }
-
-    if (CVPixelBufferIsPlanar(pixbuf)) {
-
-        planes = CVPixelBufferGetPlaneCount(pixbuf);
-        for (i = 0; i < planes; i++) {
-            data[i]     = CVPixelBufferGetBaseAddressOfPlane(pixbuf, i);
-            linesize[i] = CVPixelBufferGetBytesPerRowOfPlane(pixbuf, i);
-        }
-    } else {
-        data[0] = CVPixelBufferGetBaseAddress(pixbuf);
-        linesize[0] = CVPixelBufferGetBytesPerRow(pixbuf);
-    }
-
-    av_image_copy(vt->tmp_frame->data, vt->tmp_frame->linesize,
-                  (const uint8_t **)data, linesize, vt->tmp_frame->format,
-                  frame->width, frame->height);
-
-    ret = av_frame_copy_props(vt->tmp_frame, frame);
-    CVPixelBufferUnlockBaseAddress(pixbuf, kCVPixelBufferLock_ReadOnly);
-    if (ret < 0)
-        return ret;
-
-    av_frame_unref(frame);
-    av_frame_move_ref(frame, vt->tmp_frame);
-
-    return 0;
-}
-
-static void videotoolbox_uninit(AVCodecContext *s)
-{
-    InputStream *ist = s->opaque;
-    VTContext  *vt = ist->hwaccel_ctx;
-
-    ist->hwaccel_uninit        = NULL;
-    ist->hwaccel_retrieve_data = NULL;
-
-    av_frame_free(&vt->tmp_frame);
-
-    av_videotoolbox_default_free(s);
-    av_freep(&ist->hwaccel_ctx);
-}
-
-int videotoolbox_init(AVCodecContext *s)
-{
-    InputStream *ist = s->opaque;
-    int loglevel = (ist->hwaccel_id == HWACCEL_AUTO) ? AV_LOG_VERBOSE : AV_LOG_ERROR;
-    int ret = 0;
-    VTContext *vt;
-
-    vt = av_mallocz(sizeof(*vt));
-    if (!vt)
-        return AVERROR(ENOMEM);
-
-    ist->hwaccel_ctx           = vt;
-    ist->hwaccel_uninit        = videotoolbox_uninit;
-    ist->hwaccel_retrieve_data = videotoolbox_retrieve_data;
-
-    vt->tmp_frame = av_frame_alloc();
-    if (!vt->tmp_frame) {
-        ret = AVERROR(ENOMEM);
-        goto fail;
-    }
-
-    // TODO: reindent
-        if (!videotoolbox_pixfmt) {
-            ret = av_videotoolbox_default_init(s);
-        } else {
-            AVVideotoolboxContext *vtctx = av_videotoolbox_alloc_context();
-            CFStringRef pixfmt_str = CFStringCreateWithCString(kCFAllocatorDefault,
-                                                               videotoolbox_pixfmt,
-                                                               kCFStringEncodingUTF8);
-#if HAVE_UTGETOSTYPEFROMSTRING
-            vtctx->cv_pix_fmt_type = UTGetOSTypeFromString(pixfmt_str);
-#else
-            av_log(s, loglevel, "UTGetOSTypeFromString() is not available "
-                   "on this platform, %s pixel format can not be honored from "
-                   "the command line\n", videotoolbox_pixfmt);
-#endif
-            ret = av_videotoolbox_default_init2(s, vtctx);
-            CFRelease(pixfmt_str);
-        }
-    if (ret < 0) {
-        av_log(NULL, loglevel, "Error creating Videotoolbox decoder.\n");
-        goto fail;
-    }
-
-    return 0;
-fail:
-    videotoolbox_uninit(s);
-    return ret;
-}