[FFmpeg-devel] [PATCH] avformat/mp3dec: fix gapless audio support

2014-09-20 Thread wm4
The code already had skipping of initial padding, but discarding
trailing frame padding was missing.

This is somewhat questionable, because it will make the decoder discard
any data after the declared file size in the LAME header. But note that
skipping full frames at the end of the stream is required. Encoders
actually create such files.

---
By the way, can someone explain to me why mp3dec adjusts st-start_time?
It looks like this will be propagated to AVFormatContext.start_time. But
I would expect that the start time of mp3s is always 0, and that
libavcodec does _not_ change the timestamp of the first packet.
---
 libavformat/avformat.h |  8 
 libavformat/mp3dec.c   |  1 +
 libavformat/utils.c| 17 -
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index b915148..2370cb0 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1031,6 +1031,14 @@ typedef struct AVStream {
 int skip_samples;
 
 /**
+ * If not 0, the first audio sample that should be discarded from the 
stream.
+ * This is broken by design (needs global sample count), but can't be
+ * avoided for broken by design formats such as mp3 with ad-hoc gapless
+ * audio support.
+ */
+int64_t end_discard_sample;
+
+/**
  * Number of internally decoded frames, used internally in libavformat, do 
not access
  * its lifetime differs from info which is why it is not in that structure.
  */
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 688f235..25a2c1b 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -219,6 +219,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream 
*st,
 mp3-start_pad = v12;
 mp3-  end_pad = v4095;
 st-skip_samples = mp3-start_pad + 528 + 1;
+st-end_discard_sample = -mp3-end_pad  + 528 + 1 + mp3-frames * 
(int64_t)spf;
 if (!st-start_time)
 st-start_time = av_rescale_q(st-skip_samples,
 (AVRational){1, c-sample_rate},
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 6e828f7..f8015cc 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1255,6 +1255,11 @@ static int read_from_packet_buffer(AVPacketList 
**pkt_buffer,
 return 0;
 }
 
+static int64_t ts_to_samples(AVStream *st, int64_t ts)
+{
+return av_rescale(ts, st-time_base.num * st-codec-sample_rate, 
st-time_base.den);
+}
+
 static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
 {
 int ret = 0, i, got_packet = 0;
@@ -1352,10 +1357,20 @@ static int read_frame_internal(AVFormatContext *s, 
AVPacket *pkt)
 
 if (ret = 0) {
 AVStream *st = s-streams[pkt-stream_index];
-if (st-skip_samples) {
+int discard_padding = 0;
+if (st-end_discard_sample  pkt-pts != AV_NOPTS_VALUE) {
+int64_t pts = pkt-pts - (is_relative(pkt-pts) ? RELATIVE_TS_BASE 
: 0);
+int64_t sample = ts_to_samples(st, pts);
+int duration = ts_to_samples(st, pkt-duration);
+int64_t end_sample = sample + duration;
+if (duration  0  end_sample = st-end_discard_sample)
+discard_padding = FFMIN(end_sample - st-end_discard_sample, 
duration);
+}
+if (st-skip_samples || discard_padding) {
 uint8_t *p = av_packet_new_side_data(pkt, 
AV_PKT_DATA_SKIP_SAMPLES, 10);
 if (p) {
 AV_WL32(p, st-skip_samples);
+AV_WL32(p + 4, discard_padding);
 av_log(s, AV_LOG_DEBUG, demuxer injecting skip %d\n, 
st-skip_samples);
 }
 st-skip_samples = 0;
-- 
2.1.0

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


Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: fix gapless audio support

2014-09-20 Thread Michael Niedermayer
On Sat, Sep 20, 2014 at 11:10:19AM +0200, wm4 wrote:
 The code already had skipping of initial padding, but discarding
 trailing frame padding was missing.
 
 This is somewhat questionable, because it will make the decoder discard
 any data after the declared file size in the LAME header. But note that
 skipping full frames at the end of the stream is required. Encoders
 actually create such files.
 
 ---
 By the way, can someone explain to me why mp3dec adjusts st-start_time?
 It looks like this will be propagated to AVFormatContext.start_time. But
 I would expect that the start time of mp3s is always 0, and that
 libavcodec does _not_ change the timestamp of the first packet.
 ---
  libavformat/avformat.h |  8 
  libavformat/mp3dec.c   |  1 +
  libavformat/utils.c| 17 -
  3 files changed, 25 insertions(+), 1 deletion(-)

breaks fate-exif-image-embedded
--- ./tests/ref/fate/exif-image-embedded2014-09-19 16:37:27.026513665 
+0200
+++ tests/data/fate/exif-image-embedded 2014-09-20 12:48:39.308044761 +0200
@@ -42,363 +42,3 @@
 channels=2
 channel_layout=stereo
 [/FRAME]
-[FRAME]
-media_type=audio
-key_frame=1
-pkt_pts=368640
-pkt_pts_time=0.026122
-pkt_dts=368640
-pkt_dts_time=0.026122
-best_effort_timestamp=368640
-best_effort_timestamp_time=0.026122
-pkt_duration=368640
-pkt_duration_time=0.026122
-pkt_pos=16709
-pkt_size=418
-sample_fmt=s16p
-nb_samples=1152
...


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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.


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


[FFmpeg-devel] [PATCH] avformat/mp3dec: fix gapless audio support

2014-09-20 Thread wm4
The code already had skipping of initial padding, but discarding
trailing frame padding was missing.

This is somewhat questionable, because it will make the decoder discard
any data after the declared file size in the LAME header. But note that
skipping full frames at the end of the stream is required. Encoders
actually create such files.
---
Fix exif-image-embedded: don't set bogus end_discard_sample if the
Xing headers don't contain a frame count.
---
 libavformat/avformat.h |  8 
 libavformat/mp3dec.c   |  2 ++
 libavformat/utils.c| 17 -
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index b915148..2370cb0 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -1031,6 +1031,14 @@ typedef struct AVStream {
 int skip_samples;
 
 /**
+ * If not 0, the first audio sample that should be discarded from the 
stream.
+ * This is broken by design (needs global sample count), but can't be
+ * avoided for broken by design formats such as mp3 with ad-hoc gapless
+ * audio support.
+ */
+int64_t end_discard_sample;
+
+/**
  * Number of internally decoded frames, used internally in libavformat, do 
not access
  * its lifetime differs from info which is why it is not in that structure.
  */
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 688f235..ade1677 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -219,6 +219,8 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream 
*st,
 mp3-start_pad = v12;
 mp3-  end_pad = v4095;
 st-skip_samples = mp3-start_pad + 528 + 1;
+if (mp3-frames)
+st-end_discard_sample = -mp3-end_pad + 528 + 1 + mp3-frames * 
(int64_t)spf;
 if (!st-start_time)
 st-start_time = av_rescale_q(st-skip_samples,
 (AVRational){1, c-sample_rate},
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 6e828f7..f8015cc 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -1255,6 +1255,11 @@ static int read_from_packet_buffer(AVPacketList 
**pkt_buffer,
 return 0;
 }
 
+static int64_t ts_to_samples(AVStream *st, int64_t ts)
+{
+return av_rescale(ts, st-time_base.num * st-codec-sample_rate, 
st-time_base.den);
+}
+
 static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
 {
 int ret = 0, i, got_packet = 0;
@@ -1352,10 +1357,20 @@ static int read_frame_internal(AVFormatContext *s, 
AVPacket *pkt)
 
 if (ret = 0) {
 AVStream *st = s-streams[pkt-stream_index];
-if (st-skip_samples) {
+int discard_padding = 0;
+if (st-end_discard_sample  pkt-pts != AV_NOPTS_VALUE) {
+int64_t pts = pkt-pts - (is_relative(pkt-pts) ? RELATIVE_TS_BASE 
: 0);
+int64_t sample = ts_to_samples(st, pts);
+int duration = ts_to_samples(st, pkt-duration);
+int64_t end_sample = sample + duration;
+if (duration  0  end_sample = st-end_discard_sample)
+discard_padding = FFMIN(end_sample - st-end_discard_sample, 
duration);
+}
+if (st-skip_samples || discard_padding) {
 uint8_t *p = av_packet_new_side_data(pkt, 
AV_PKT_DATA_SKIP_SAMPLES, 10);
 if (p) {
 AV_WL32(p, st-skip_samples);
+AV_WL32(p + 4, discard_padding);
 av_log(s, AV_LOG_DEBUG, demuxer injecting skip %d\n, 
st-skip_samples);
 }
 st-skip_samples = 0;
-- 
2.1.0

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


Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: fix gapless audio support

2014-09-20 Thread Michael Niedermayer
On Sat, Sep 20, 2014 at 01:48:05PM +0200, wm4 wrote:
 The code already had skipping of initial padding, but discarding
 trailing frame padding was missing.
 
 This is somewhat questionable, because it will make the decoder discard
 any data after the declared file size in the LAME header. But note that
 skipping full frames at the end of the stream is required. Encoders
 actually create such files.
 ---
 Fix exif-image-embedded: don't set bogus end_discard_sample if the
 Xing headers don't contain a frame count.
 ---
  libavformat/avformat.h |  8 
  libavformat/mp3dec.c   |  2 ++
  libavformat/utils.c| 17 -
  3 files changed, 26 insertions(+), 1 deletion(-)

applied

thanks

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

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes


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