Re: [FFmpeg-devel] [PATCH v4 3/3] aadec: improve seeking in mp3 content

2018-07-11 Thread Karsten Otto

> Am 11.07.2018 um 20:09 schrieb Michael Niedermayer :
> 
> Signierter PGP-Teil
> On Sat, Jul 07, 2018 at 07:41:29PM +0200, Karsten Otto wrote:
>> MP3 frames may not be aligned to aa chunk boundaries. When seeking,
>> calculate the expected frame offset in the target chunk. Adjust the
>> timestamp and truncate the next packet accordingly.
>> 
>> This solution works for the majority of tested audio material. For
>> some rare encodings with mp3 padding or embedded id3 tags, it will
>> mispredict the correct offset, and at worst skip an extra frame.
>> ---
>> libavformat/aadec.c | 15 ---
>> 1 file changed, 12 insertions(+), 3 deletions(-)
>> 
>> diff --git a/libavformat/aadec.c b/libavformat/aadec.c
>> index e3c03bc522..2b9e4e526c 100644
>> --- a/libavformat/aadec.c
>> +++ b/libavformat/aadec.c
>> @@ -37,6 +37,7 @@
>> #define TEA_BLOCK_SIZE 8
>> #define CHAPTER_HEADER_SIZE 8
>> #define TIMEPREC 1000
>> +#define MP3_FRAME_SIZE 104
>> 
>> typedef struct AADemuxContext {
>> AVClass *class;
>> @@ -50,6 +51,7 @@ typedef struct AADemuxContext {
>> int64_t current_chapter_size;
>> int64_t content_start;
>> int64_t content_end;
>> +int seek_offset;
>> } AADemuxContext;
>> 
>> static int get_second_size(char *codec_name)
>> @@ -230,6 +232,7 @@ static int aa_read_header(AVFormatContext *s)
>> ff_update_cur_dts(s, st, 0);
>> avio_seek(pb, start, SEEK_SET);
>> c->current_chapter_size = 0;
>> +c->seek_offset = 0;
>> 
>> return 0;
>> }
>> @@ -295,12 +298,13 @@ static int aa_read_packet(AVFormatContext *s, AVPacket 
>> *pkt)
>> if (c->current_chapter_size <= 0)
>> c->current_chapter_size = 0;
>> 
>> -ret = av_new_packet(pkt, written);
>> +ret = av_new_packet(pkt, written - c->seek_offset);
>> if (ret < 0)
>> return ret;
>> -memcpy(pkt->data, buf, written);
>> +memcpy(pkt->data, buf + c->seek_offset, written - c->seek_offset);
> 
> what if written < c->seek_offset ? or is this impossible ?
> 
In the normal case this cannot happen, as each chapter is a multiple of the
frame size. They could be equal, which would result in an empty packet.
But I am not quite sure about the worst case I mentioned; to be on the safe
side, I can add an extra check for the last chapter block. Will send a new
patch version.

Cheers, Karsten

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v4 3/3] aadec: improve seeking in mp3 content

2018-07-11 Thread Michael Niedermayer
On Sat, Jul 07, 2018 at 07:41:29PM +0200, Karsten Otto wrote:
> MP3 frames may not be aligned to aa chunk boundaries. When seeking,
> calculate the expected frame offset in the target chunk. Adjust the
> timestamp and truncate the next packet accordingly.
> 
> This solution works for the majority of tested audio material. For
> some rare encodings with mp3 padding or embedded id3 tags, it will
> mispredict the correct offset, and at worst skip an extra frame.
> ---
>  libavformat/aadec.c | 15 ---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/aadec.c b/libavformat/aadec.c
> index e3c03bc522..2b9e4e526c 100644
> --- a/libavformat/aadec.c
> +++ b/libavformat/aadec.c
> @@ -37,6 +37,7 @@
>  #define TEA_BLOCK_SIZE 8
>  #define CHAPTER_HEADER_SIZE 8
>  #define TIMEPREC 1000
> +#define MP3_FRAME_SIZE 104
>  
>  typedef struct AADemuxContext {
>  AVClass *class;
> @@ -50,6 +51,7 @@ typedef struct AADemuxContext {
>  int64_t current_chapter_size;
>  int64_t content_start;
>  int64_t content_end;
> +int seek_offset;
>  } AADemuxContext;
>  
>  static int get_second_size(char *codec_name)
> @@ -230,6 +232,7 @@ static int aa_read_header(AVFormatContext *s)
>  ff_update_cur_dts(s, st, 0);
>  avio_seek(pb, start, SEEK_SET);
>  c->current_chapter_size = 0;
> +c->seek_offset = 0;
>  
>  return 0;
>  }
> @@ -295,12 +298,13 @@ static int aa_read_packet(AVFormatContext *s, AVPacket 
> *pkt)
>  if (c->current_chapter_size <= 0)
>  c->current_chapter_size = 0;
>  
> -ret = av_new_packet(pkt, written);
> +ret = av_new_packet(pkt, written - c->seek_offset);
>  if (ret < 0)
>  return ret;
> -memcpy(pkt->data, buf, written);
> +memcpy(pkt->data, buf + c->seek_offset, written - c->seek_offset);

what if written < c->seek_offset ? or is this impossible ?

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH v4 3/3] aadec: improve seeking in mp3 content

2018-07-10 Thread Karsten Otto
Ping - What about this one? I tested it with about 20 files and it works 
perfectly
for all of them - except one which has tag/padding. In its case, playback 
quality
is virtually the same as without the patch, i.e. no harm done.

Cheers, Karsten

> Am 07.07.2018 um 19:41 schrieb Karsten Otto :
> 
> MP3 frames may not be aligned to aa chunk boundaries. When seeking,
> calculate the expected frame offset in the target chunk. Adjust the
> timestamp and truncate the next packet accordingly.
> 
> This solution works for the majority of tested audio material. For
> some rare encodings with mp3 padding or embedded id3 tags, it will
> mispredict the correct offset, and at worst skip an extra frame.
> ---
> libavformat/aadec.c | 15 ---
> 1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/aadec.c b/libavformat/aadec.c
> index e3c03bc522..2b9e4e526c 100644
> --- a/libavformat/aadec.c
> +++ b/libavformat/aadec.c
> @@ -37,6 +37,7 @@
> #define TEA_BLOCK_SIZE 8
> #define CHAPTER_HEADER_SIZE 8
> #define TIMEPREC 1000
> +#define MP3_FRAME_SIZE 104
> 
> typedef struct AADemuxContext {
> AVClass *class;
> @@ -50,6 +51,7 @@ typedef struct AADemuxContext {
> int64_t current_chapter_size;
> int64_t content_start;
> int64_t content_end;
> +int seek_offset;
> } AADemuxContext;
> 
> static int get_second_size(char *codec_name)
> @@ -230,6 +232,7 @@ static int aa_read_header(AVFormatContext *s)
> ff_update_cur_dts(s, st, 0);
> avio_seek(pb, start, SEEK_SET);
> c->current_chapter_size = 0;
> +c->seek_offset = 0;
> 
> return 0;
> }
> @@ -295,12 +298,13 @@ static int aa_read_packet(AVFormatContext *s, AVPacket 
> *pkt)
> if (c->current_chapter_size <= 0)
> c->current_chapter_size = 0;
> 
> -ret = av_new_packet(pkt, written);
> +ret = av_new_packet(pkt, written - c->seek_offset);
> if (ret < 0)
> return ret;
> -memcpy(pkt->data, buf, written);
> +memcpy(pkt->data, buf + c->seek_offset, written - c->seek_offset);
> pkt->pos = pos;
> 
> +c->seek_offset = 0;
> return 0;
> }
> 
> @@ -344,7 +348,12 @@ static int aa_read_seek(AVFormatContext *s,
> c->current_chapter_size = chapter_size - chapter_pos;
> c->chapter_idx = 1 + chapter_idx;
> 
> -ff_update_cur_dts(s, s->streams[0], ch->start + chapter_pos * TIMEPREC);
> +// handle extra offset for unaligned frames
> +if (s->streams[0]->codecpar->codec_id == AV_CODEC_ID_MP3) {
> +c->seek_offset = (MP3_FRAME_SIZE - chapter_pos % MP3_FRAME_SIZE) % 
> MP3_FRAME_SIZE;
> +}
> +
> +ff_update_cur_dts(s, s->streams[0], ch->start + (chapter_pos + 
> c->seek_offset) * TIMEPREC);
> 
> return 1;
> }
> -- 
> 2.14.3 (Apple Git-98)
> 
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel