diff mbox series

[FFmpeg-devel] avcodec/bsf/mp3_header_decompress: Remove BSF

Message ID AS8P250MB074471F9EB218B0BFDD31F828F582@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM
State Accepted
Headers show
Series [FFmpeg-devel] avcodec/bsf/mp3_header_decompress: Remove BSF | expand

Checks

Context Check Description
yinshiyou/commit_msg_loongarch64 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/commit_msg_x86 warning The first line of the commit message must start with a context terminated by a colon and a space, for example "lavu/opt: " or "doc: ".
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Andreas Rheinhardt Feb. 28, 2024, 4:13 p.m. UTC
This BSF is supposed to be used in conjunction with mp3_header_compress,
which has been removed more than ten years ago in commit
c6080d89009056530119ab794ad02e4d515c7754. It mangled the headers
by removing the CRC field as well as fields that are supposed
to stay constant for the entirety of a stream (which are put into
extradata). This made these files unplayable; they need to be
decompressed with the BSF first (which does not happen automatically).
Even in this case the CRC does not get restored.

I am not aware that such compressed files exist at all; therefore
this commit removes the BSF completely.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 doc/bitstream_filters.texi             |   4 -
 libavcodec/Makefile                    |   1 -
 libavcodec/bitstream_filters.c         |   1 -
 libavcodec/bsf/Makefile                |   1 -
 libavcodec/bsf/mp3_header_decompress.c | 130 -------------------------
 5 files changed, 137 deletions(-)
 delete mode 100644 libavcodec/bsf/mp3_header_decompress.c

Comments

Andreas Rheinhardt March 1, 2024, 11:34 a.m. UTC | #1
Andreas Rheinhardt:
> This BSF is supposed to be used in conjunction with mp3_header_compress,
> which has been removed more than ten years ago in commit
> c6080d89009056530119ab794ad02e4d515c7754. It mangled the headers
> by removing the CRC field as well as fields that are supposed
> to stay constant for the entirety of a stream (which are put into
> extradata). This made these files unplayable; they need to be
> decompressed with the BSF first (which does not happen automatically).
> Even in this case the CRC does not get restored.
> 
> I am not aware that such compressed files exist at all; therefore
> this commit removes the BSF completely.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>  doc/bitstream_filters.texi             |   4 -
>  libavcodec/Makefile                    |   1 -
>  libavcodec/bitstream_filters.c         |   1 -
>  libavcodec/bsf/Makefile                |   1 -
>  libavcodec/bsf/mp3_header_decompress.c | 130 -------------------------
>  5 files changed, 137 deletions(-)
>  delete mode 100644 libavcodec/bsf/mp3_header_decompress.c
> 
> diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
> index d5bac105ff..e06de1a73a 100644
> --- a/doc/bitstream_filters.texi
> +++ b/doc/bitstream_filters.texi
> @@ -528,10 +528,6 @@ metadata header from each subtitle packet.
>  
>  See also the @ref{text2movsub} filter.
>  
> -@section mp3decomp
> -
> -Decompress non-standard compressed MP3 audio headers.
> -
>  @section mpeg2_metadata
>  
>  Modify metadata embedded in an MPEG-2 stream.
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 09ae5270b3..f28a6568de 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -1240,7 +1240,6 @@ OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)      += av1_parse.o h2645_parse.o
>  OBJS-$(CONFIG_H264_METADATA_BSF)          += h264_levels.o h2645data.o
>  OBJS-$(CONFIG_HAPQA_EXTRACT_BSF)          += hap.o
>  OBJS-$(CONFIG_HEVC_METADATA_BSF)          += h265_profile_level.o h2645data.o
> -OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mpegaudiotabs.o
>  OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += av1_parse.o
>  OBJS-$(CONFIG_TRUEHD_CORE_BSF)            += mlp_parse.o mlp.o
>  
> diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
> index 1bae113d92..12860c332b 100644
> --- a/libavcodec/bitstream_filters.c
> +++ b/libavcodec/bitstream_filters.c
> @@ -46,7 +46,6 @@ extern const FFBitStreamFilter ff_imx_dump_header_bsf;
>  extern const FFBitStreamFilter ff_media100_to_mjpegb_bsf;
>  extern const FFBitStreamFilter ff_mjpeg2jpeg_bsf;
>  extern const FFBitStreamFilter ff_mjpega_dump_header_bsf;
> -extern const FFBitStreamFilter ff_mp3_header_decompress_bsf;
>  extern const FFBitStreamFilter ff_mpeg2_metadata_bsf;
>  extern const FFBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
>  extern const FFBitStreamFilter ff_mov2textsub_bsf;
> diff --git a/libavcodec/bsf/Makefile b/libavcodec/bsf/Makefile
> index 62609eb24e..fb70ad0c21 100644
> --- a/libavcodec/bsf/Makefile
> +++ b/libavcodec/bsf/Makefile
> @@ -25,7 +25,6 @@ OBJS-$(CONFIG_MEDIA100_TO_MJPEGB_BSF)     += bsf/media100_to_mjpegb.o
>  OBJS-$(CONFIG_MJPEG2JPEG_BSF)             += bsf/mjpeg2jpeg.o
>  OBJS-$(CONFIG_MJPEGA_DUMP_HEADER_BSF)     += bsf/mjpega_dump_header.o
>  OBJS-$(CONFIG_MOV2TEXTSUB_BSF)            += bsf/movsub.o
> -OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += bsf/mp3_header_decompress.o
>  OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += bsf/mpeg2_metadata.o
>  OBJS-$(CONFIG_MPEG4_UNPACK_BFRAMES_BSF)   += bsf/mpeg4_unpack_bframes.o
>  OBJS-$(CONFIG_NOISE_BSF)                  += bsf/noise.o
> diff --git a/libavcodec/bsf/mp3_header_decompress.c b/libavcodec/bsf/mp3_header_decompress.c
> deleted file mode 100644
> index a177029fe0..0000000000
> --- a/libavcodec/bsf/mp3_header_decompress.c
> +++ /dev/null
> @@ -1,130 +0,0 @@
> -/*
> - * copyright (c) 2006 Michael Niedermayer <michaelni@gmx.at>
> - *
> - * 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 "libavutil/common.h"
> -#include "libavutil/intreadwrite.h"
> -#include "bsf.h"
> -#include "bsf_internal.h"
> -#include "defs.h"
> -#include "mpegaudiodecheader.h"
> -#include "mpegaudiodata.h"
> -
> -
> -static int mp3_header_decompress(AVBSFContext *ctx, AVPacket *out)
> -{
> -    AVPacket *in;
> -    uint32_t header;
> -    int sample_rate= ctx->par_in->sample_rate;
> -    int sample_rate_index=0;
> -    int lsf, mpeg25, bitrate_index, frame_size, ret;
> -    uint8_t *buf;
> -    int buf_size;
> -
> -    ret = ff_bsf_get_packet(ctx, &in);
> -    if (ret < 0)
> -        return ret;
> -
> -    buf      = in->data;
> -    buf_size = in->size;
> -
> -    header = AV_RB32(buf);
> -    if(ff_mpa_check_header(header) >= 0){
> -        av_packet_move_ref(out, in);
> -        av_packet_free(&in);
> -
> -        return 0;
> -    }
> -
> -    if(ctx->par_in->extradata_size != 15 || strcmp(ctx->par_in->extradata, "FFCMP3 0.0")){
> -        av_log(ctx, AV_LOG_ERROR, "Extradata invalid %d\n", ctx->par_in->extradata_size);
> -        ret = AVERROR(EINVAL);
> -        goto fail;
> -    }
> -
> -    header= AV_RB32(ctx->par_in->extradata+11) & MP3_MASK;
> -
> -    lsf     = sample_rate < (24000+32000)/2;
> -    mpeg25  = sample_rate < (12000+16000)/2;
> -    sample_rate_index= (header>>10)&3;
> -    if (sample_rate_index == 3) {
> -        ret = AVERROR_INVALIDDATA;
> -        goto fail;
> -    }
> -
> -    sample_rate = ff_mpa_freq_tab[sample_rate_index] >> (lsf + mpeg25); //in case sample rate is a little off
> -
> -    for(bitrate_index=2; bitrate_index<30; bitrate_index++){
> -        frame_size = ff_mpa_bitrate_tab[lsf][2][bitrate_index>>1];
> -        frame_size = (frame_size * 144000) / (sample_rate << lsf) + (bitrate_index&1);
> -        if(frame_size == buf_size + 4)
> -            break;
> -        if(frame_size == buf_size + 6)
> -            break;
> -    }
> -    if(bitrate_index == 30){
> -        av_log(ctx, AV_LOG_ERROR, "Could not find bitrate_index.\n");
> -        ret = AVERROR(EINVAL);
> -        goto fail;
> -    }
> -
> -    header |= (bitrate_index&1)<<9;
> -    header |= (bitrate_index>>1)<<12;
> -    header |= (frame_size == buf_size + 4)<<16; //FIXME actually set a correct crc instead of 0
> -
> -    ret = av_new_packet(out, frame_size);
> -    if (ret < 0)
> -        goto fail;
> -    ret = av_packet_copy_props(out, in);
> -    if (ret < 0) {
> -        av_packet_unref(out);
> -        goto fail;
> -    }
> -    memcpy(out->data + frame_size - buf_size, buf, buf_size + AV_INPUT_BUFFER_PADDING_SIZE);
> -
> -    if (ctx->par_in->ch_layout.nb_channels == 2){
> -        uint8_t *p= out->data + frame_size - buf_size;
> -        if(lsf){
> -            FFSWAP(int, p[1], p[2]);
> -            header |= (p[1] & 0xC0)>>2;
> -            p[1] &= 0x3F;
> -        }else{
> -            header |= p[1] & 0x30;
> -            p[1] &= 0xCF;
> -        }
> -    }
> -
> -    AV_WB32(out->data, header);
> -
> -    ret = 0;
> -
> -fail:
> -    av_packet_free(&in);
> -    return ret;
> -}
> -
> -static const enum AVCodecID codec_ids[] = {
> -    AV_CODEC_ID_MP3, AV_CODEC_ID_NONE,
> -};
> -
> -const FFBitStreamFilter ff_mp3_header_decompress_bsf = {
> -    .p.name      = "mp3decomp",
> -    .p.codec_ids = codec_ids,
> -    .filter      = mp3_header_decompress,
> -};

Will apply tomorrow unless there are objections.

- Andreas
Anton Khirnov March 1, 2024, 3:31 p.m. UTC | #2
Quoting Andreas Rheinhardt (2024-03-01 12:34:27)
> Will apply tomorrow unless there are objections.

Looks very good to me.
diff mbox series

Patch

diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi
index d5bac105ff..e06de1a73a 100644
--- a/doc/bitstream_filters.texi
+++ b/doc/bitstream_filters.texi
@@ -528,10 +528,6 @@  metadata header from each subtitle packet.
 
 See also the @ref{text2movsub} filter.
 
-@section mp3decomp
-
-Decompress non-standard compressed MP3 audio headers.
-
 @section mpeg2_metadata
 
 Modify metadata embedded in an MPEG-2 stream.
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 09ae5270b3..f28a6568de 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1240,7 +1240,6 @@  OBJS-$(CONFIG_EXTRACT_EXTRADATA_BSF)      += av1_parse.o h2645_parse.o
 OBJS-$(CONFIG_H264_METADATA_BSF)          += h264_levels.o h2645data.o
 OBJS-$(CONFIG_HAPQA_EXTRACT_BSF)          += hap.o
 OBJS-$(CONFIG_HEVC_METADATA_BSF)          += h265_profile_level.o h2645data.o
-OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += mpegaudiotabs.o
 OBJS-$(CONFIG_REMOVE_EXTRADATA_BSF)       += av1_parse.o
 OBJS-$(CONFIG_TRUEHD_CORE_BSF)            += mlp_parse.o mlp.o
 
diff --git a/libavcodec/bitstream_filters.c b/libavcodec/bitstream_filters.c
index 1bae113d92..12860c332b 100644
--- a/libavcodec/bitstream_filters.c
+++ b/libavcodec/bitstream_filters.c
@@ -46,7 +46,6 @@  extern const FFBitStreamFilter ff_imx_dump_header_bsf;
 extern const FFBitStreamFilter ff_media100_to_mjpegb_bsf;
 extern const FFBitStreamFilter ff_mjpeg2jpeg_bsf;
 extern const FFBitStreamFilter ff_mjpega_dump_header_bsf;
-extern const FFBitStreamFilter ff_mp3_header_decompress_bsf;
 extern const FFBitStreamFilter ff_mpeg2_metadata_bsf;
 extern const FFBitStreamFilter ff_mpeg4_unpack_bframes_bsf;
 extern const FFBitStreamFilter ff_mov2textsub_bsf;
diff --git a/libavcodec/bsf/Makefile b/libavcodec/bsf/Makefile
index 62609eb24e..fb70ad0c21 100644
--- a/libavcodec/bsf/Makefile
+++ b/libavcodec/bsf/Makefile
@@ -25,7 +25,6 @@  OBJS-$(CONFIG_MEDIA100_TO_MJPEGB_BSF)     += bsf/media100_to_mjpegb.o
 OBJS-$(CONFIG_MJPEG2JPEG_BSF)             += bsf/mjpeg2jpeg.o
 OBJS-$(CONFIG_MJPEGA_DUMP_HEADER_BSF)     += bsf/mjpega_dump_header.o
 OBJS-$(CONFIG_MOV2TEXTSUB_BSF)            += bsf/movsub.o
-OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF)  += bsf/mp3_header_decompress.o
 OBJS-$(CONFIG_MPEG2_METADATA_BSF)         += bsf/mpeg2_metadata.o
 OBJS-$(CONFIG_MPEG4_UNPACK_BFRAMES_BSF)   += bsf/mpeg4_unpack_bframes.o
 OBJS-$(CONFIG_NOISE_BSF)                  += bsf/noise.o
diff --git a/libavcodec/bsf/mp3_header_decompress.c b/libavcodec/bsf/mp3_header_decompress.c
deleted file mode 100644
index a177029fe0..0000000000
--- a/libavcodec/bsf/mp3_header_decompress.c
+++ /dev/null
@@ -1,130 +0,0 @@ 
-/*
- * copyright (c) 2006 Michael Niedermayer <michaelni@gmx.at>
- *
- * 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 "libavutil/common.h"
-#include "libavutil/intreadwrite.h"
-#include "bsf.h"
-#include "bsf_internal.h"
-#include "defs.h"
-#include "mpegaudiodecheader.h"
-#include "mpegaudiodata.h"
-
-
-static int mp3_header_decompress(AVBSFContext *ctx, AVPacket *out)
-{
-    AVPacket *in;
-    uint32_t header;
-    int sample_rate= ctx->par_in->sample_rate;
-    int sample_rate_index=0;
-    int lsf, mpeg25, bitrate_index, frame_size, ret;
-    uint8_t *buf;
-    int buf_size;
-
-    ret = ff_bsf_get_packet(ctx, &in);
-    if (ret < 0)
-        return ret;
-
-    buf      = in->data;
-    buf_size = in->size;
-
-    header = AV_RB32(buf);
-    if(ff_mpa_check_header(header) >= 0){
-        av_packet_move_ref(out, in);
-        av_packet_free(&in);
-
-        return 0;
-    }
-
-    if(ctx->par_in->extradata_size != 15 || strcmp(ctx->par_in->extradata, "FFCMP3 0.0")){
-        av_log(ctx, AV_LOG_ERROR, "Extradata invalid %d\n", ctx->par_in->extradata_size);
-        ret = AVERROR(EINVAL);
-        goto fail;
-    }
-
-    header= AV_RB32(ctx->par_in->extradata+11) & MP3_MASK;
-
-    lsf     = sample_rate < (24000+32000)/2;
-    mpeg25  = sample_rate < (12000+16000)/2;
-    sample_rate_index= (header>>10)&3;
-    if (sample_rate_index == 3) {
-        ret = AVERROR_INVALIDDATA;
-        goto fail;
-    }
-
-    sample_rate = ff_mpa_freq_tab[sample_rate_index] >> (lsf + mpeg25); //in case sample rate is a little off
-
-    for(bitrate_index=2; bitrate_index<30; bitrate_index++){
-        frame_size = ff_mpa_bitrate_tab[lsf][2][bitrate_index>>1];
-        frame_size = (frame_size * 144000) / (sample_rate << lsf) + (bitrate_index&1);
-        if(frame_size == buf_size + 4)
-            break;
-        if(frame_size == buf_size + 6)
-            break;
-    }
-    if(bitrate_index == 30){
-        av_log(ctx, AV_LOG_ERROR, "Could not find bitrate_index.\n");
-        ret = AVERROR(EINVAL);
-        goto fail;
-    }
-
-    header |= (bitrate_index&1)<<9;
-    header |= (bitrate_index>>1)<<12;
-    header |= (frame_size == buf_size + 4)<<16; //FIXME actually set a correct crc instead of 0
-
-    ret = av_new_packet(out, frame_size);
-    if (ret < 0)
-        goto fail;
-    ret = av_packet_copy_props(out, in);
-    if (ret < 0) {
-        av_packet_unref(out);
-        goto fail;
-    }
-    memcpy(out->data + frame_size - buf_size, buf, buf_size + AV_INPUT_BUFFER_PADDING_SIZE);
-
-    if (ctx->par_in->ch_layout.nb_channels == 2){
-        uint8_t *p= out->data + frame_size - buf_size;
-        if(lsf){
-            FFSWAP(int, p[1], p[2]);
-            header |= (p[1] & 0xC0)>>2;
-            p[1] &= 0x3F;
-        }else{
-            header |= p[1] & 0x30;
-            p[1] &= 0xCF;
-        }
-    }
-
-    AV_WB32(out->data, header);
-
-    ret = 0;
-
-fail:
-    av_packet_free(&in);
-    return ret;
-}
-
-static const enum AVCodecID codec_ids[] = {
-    AV_CODEC_ID_MP3, AV_CODEC_ID_NONE,
-};
-
-const FFBitStreamFilter ff_mp3_header_decompress_bsf = {
-    .p.name      = "mp3decomp",
-    .p.codec_ids = codec_ids,
-    .filter      = mp3_header_decompress,
-};