Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-24 Thread Carl Eugen Hoyos 
On Saturday 19 July 2014 04:40:47 pm Joakim Plate wrote:
 Carl Eugen Hoyos cehoyos at ag.or.at writes:
  Hi!
 
  Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.

 What about streaming growing files? It's pretty good to know number of
 frames of the complete mp3 at start of playback?

This is true (ticket #2590 contains an example).

New patch attached.

Please review, Carl Eugen
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 8335388..4016e94 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -138,6 +138,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream 
*st,
 
 MP3DecContext *mp3 = s-priv_data;
 static const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
+uint64_t min, delta, fsize;
 
 /* Check for Xing / Info tag */
 avio_skip(s-pb, xing_offtbl[c-lsf == 1][c-nb_channels == 1]);
@@ -151,6 +152,15 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
AVStream *st,
 mp3-frames = avio_rb32(s-pb);
 if (v  XING_FLAG_SIZE)
 mp3-header_filesize = avio_rb32(s-pb);
+fsize = avio_size(s-pb);
+min = FFMIN(fsize, mp3-header_filesize);
+delta = FFMAX(fsize, mp3-header_filesize) - min;
+if (fsize  fsize  mp3-header_filesize  delta  min  4) {
+mp3-frames = 0;
+} else if (fsize  delta  min  4) {
+av_log(s, AV_LOG_WARNING,
+   filesize and duration do not match (growing file?)\n);
+}
 if (v  XING_FLAG_TOC)
 read_xing_toc(s, mp3-header_filesize, av_rescale_q(mp3-frames,
(AVRational){spf, c-sample_rate},
@@ -399,7 +409,10 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 int i, j;
 int dir = (flagsAVSEEK_FLAG_BACKWARD) ? -1 : 1;
 
-if (mp3-is_cbr  st-duration  0  mp3-header_filesize  
s-data_offset) {
+if (   mp3-is_cbr
+ st-duration  0
+ mp3-header_filesize  s-data_offset
+ mp3-frames) {
 int64_t filesize = avio_size(s-pb);
 int64_t duration;
 if (filesize = s-data_offset)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is?wrong

2014-07-24 Thread Michael Niedermayer 
On Thu, Jul 24, 2014 at 12:50:55PM +0200, Carl Eugen Hoyos wrote:
 On Saturday 19 July 2014 04:40:47 pm Joakim Plate wrote:
  Carl Eugen Hoyos cehoyos at ag.or.at writes:
   Hi!
  
   Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.
 
  What about streaming growing files? It's pretty good to know number of
  frames of the complete mp3 at start of playback?
 
 This is true (ticket #2590 contains an example).
 
 New patch attached.
 
 Please review, Carl Eugen

  mp3dec.c |   15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)
 e354c8374f6f9655a2a4c1904636218cc4bc9ee6  patchmp3duration2.diff
 diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
 index 8335388..4016e94 100644

breaks fate

stddev: 6864.49 PSNR: 19.60 MAXDIFF:40809 bytes:   882432/   884736
Test mp3-float-extra_overread failed. Look at 
tests/data/fate/mp3-float-extra_overread.err for details.
make: *** [fate-mp3-float-extra_overread] Error 1

[...]

-- 
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

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-24 Thread Carl Eugen Hoyos 
On Thursday 24 July 2014 03:00:02 pm Michael Niedermayer wrote:
 On Thu, Jul 24, 2014 at 12:50:55PM +0200, Carl Eugen Hoyos wrote:
  On Saturday 19 July 2014 04:40:47 pm Joakim Plate wrote:
   Carl Eugen Hoyos cehoyos at ag.or.at writes:
Hi!
   
Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.
  
   What about streaming growing files? It's pretty good to know number of
   frames of the complete mp3 at start of playback?
 
  This is true (ticket #2590 contains an example).
 
  New patch attached.
 
  Please review, Carl Eugen
 
   mp3dec.c |   15 ++-
   1 file changed, 14 insertions(+), 1 deletion(-)
  e354c8374f6f9655a2a4c1904636218cc4bc9ee6  patchmp3duration2.diff
  diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
  index 8335388..4016e94 100644

 breaks fate

New patch attached.

Thank you, Carl Eugen
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 8335388..688f235 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -138,6 +138,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream 
*st,
 
 MP3DecContext *mp3 = s-priv_data;
 static const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
+uint64_t fsize = avio_size(s-pb);
 
 /* Check for Xing / Info tag */
 avio_skip(s-pb, xing_offtbl[c-lsf == 1][c-nb_channels == 1]);
@@ -151,6 +152,17 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
AVStream *st,
 mp3-frames = avio_rb32(s-pb);
 if (v  XING_FLAG_SIZE)
 mp3-header_filesize = avio_rb32(s-pb);
+if (fsize  mp3-header_filesize) {
+uint64_t min, delta;
+min = FFMIN(fsize, mp3-header_filesize);
+delta = FFMAX(fsize, mp3-header_filesize) - min;
+if (fsize  mp3-header_filesize  delta  min  4) {
+mp3-frames = 0;
+} else if (delta  min  4) {
+av_log(s, AV_LOG_WARNING,
+   filesize and duration do not match (growing file?)\n);
+}
+}
 if (v  XING_FLAG_TOC)
 read_xing_toc(s, mp3-header_filesize, av_rescale_q(mp3-frames,
(AVRational){spf, c-sample_rate},
@@ -399,7 +411,10 @@ static int mp3_seek(AVFormatContext *s, int stream_index, 
int64_t timestamp,
 int i, j;
 int dir = (flagsAVSEEK_FLAG_BACKWARD) ? -1 : 1;
 
-if (mp3-is_cbr  st-duration  0  mp3-header_filesize  
s-data_offset) {
+if (   mp3-is_cbr
+ st-duration  0
+ mp3-header_filesize  s-data_offset
+ mp3-frames) {
 int64_t filesize = avio_size(s-pb);
 int64_t duration;
 if (filesize = s-data_offset)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-24 Thread Michael Niedermayer 
On Thu, Jul 24, 2014 at 03:15:56PM +0200, Carl Eugen Hoyos wrote:
 On Thursday 24 July 2014 03:00:02 pm Michael Niedermayer wrote:
  On Thu, Jul 24, 2014 at 12:50:55PM +0200, Carl Eugen Hoyos wrote:
   On Saturday 19 July 2014 04:40:47 pm Joakim Plate wrote:
Carl Eugen Hoyos cehoyos at ag.or.at writes:
 Hi!

 Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.
   
What about streaming growing files? It's pretty good to know number of
frames of the complete mp3 at start of playback?
  
   This is true (ticket #2590 contains an example).
  
   New patch attached.
  
   Please review, Carl Eugen
  
mp3dec.c |   15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
   e354c8374f6f9655a2a4c1904636218cc4bc9ee6  patchmp3duration2.diff
   diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
   index 8335388..4016e94 100644
 
  breaks fate
 
 New patch attached.
 
 Thank you, Carl Eugen

  mp3dec.c |   17 -
  1 file changed, 16 insertions(+), 1 deletion(-)
 ad2e4e19110a314cbcf1c97640940846c15c85a0  patchmp3duration3.diff
 diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
 index 8335388..688f235 100644

probably ok

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

DNS cache poisoning attacks, popular search engine, Google internet authority
dont be evil, please


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

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-19 Thread Joakim Plate 
Carl Eugen Hoyos cehoyos at ag.or.at writes:

 
 Hi!
 
 Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.
 
 Please comment, Carl Eugen
 

What about streaming growing files? It's pretty good to know number of frames
of the complete mp3 at start of playback?

/Joakim


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

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-17 Thread Nicolas George 
Le nonidi 29 messidor, an CCXXII, Carl Eugen Hoyos a écrit :
 Hi!
 
 Attached patch fixes ticket #3777 for me, analyzed by Oliver Fromme.
 
 Please comment, Carl Eugen

 diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
 index 8335388..cdef6d9 100644
 --- a/libavformat/mp3dec.c
 +++ b/libavformat/mp3dec.c
 @@ -138,6 +138,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
 AVStream *st,
  
  MP3DecContext *mp3 = s-priv_data;
  static const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
 +int64_t fsize = avio_size(s-pb);
  
  /* Check for Xing / Info tag */
  avio_skip(s-pb, xing_offtbl[c-lsf == 1][c-nb_channels == 1]);
 @@ -151,6 +152,9 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
 AVStream *st,
  mp3-frames = avio_rb32(s-pb);
  if (v  XING_FLAG_SIZE)
  mp3-header_filesize = avio_rb32(s-pb);

 +if (fsize  0  mp3-header_filesize  0 
 +FFABS(fsize - (int64_t)mp3-header_filesize) / (float)FFMIN(fsize, 
 mp3-header_filesize)  0.05)

I would suggest to avoid floating point arithmetic if possible. Possibly
something like that:

uint64_t min = FFMIN(fsize, mp3-header_filesize);
uint64_t delta = FFMAX(fsize, mp3-header_filesize) - min;
uint64_t tolerance = min / 20;
if (...  min - tolerance  2 * tolerance)

I also find this version easier to understand.

And in any case, someone may correct me, but I believe nowadays double
should always preferred to float unless you need a lot of them and want to
reduce the memory use.

I can not judge on the correctness, though.

 +mp3-frames = 0;
  if (v  XING_FLAG_TOC)
  read_xing_toc(s, mp3-header_filesize, av_rescale_q(mp3-frames,
 (AVRational){spf, c-sample_rate},

Regards,

-- 
  Nicolas George


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

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-17 Thread Carl Eugen Hoyos 
On Thursday 17 July 2014 09:41:40 pm Nicolas George wrote:
 Le nonidi 29 messidor, an CCXXII, Carl Eugen Hoyos a écrit :
 
  Attached patch fixes ticket #3777 for me, analyzed by Oliver 
  Fromme. 

 I would suggest to avoid floating point arithmetic if possible. 
 Possibly something like that:

 uint64_t min = FFMIN(fsize, mp3-header_filesize);
 uint64_t delta = FFMAX(fsize, mp3-header_filesize) - min;
 uint64_t tolerance = min / 20;
 if (...  min - tolerance  2 * tolerance)

 I also find this version easier to understand.

Thank you, new patch attached.

 I can not judge on the correctness, though.

The logic is copied from asfdec.c.

Please review, Carl Eugen
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
index 8335388..cf5aa73 100644
--- a/libavformat/mp3dec.c
+++ b/libavformat/mp3dec.c
@@ -138,6 +138,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, AVStream 
*st,
 
 MP3DecContext *mp3 = s-priv_data;
 static const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
+uint64_t min, delta, fsize = avio_size(s-pb);
 
 /* Check for Xing / Info tag */
 avio_skip(s-pb, xing_offtbl[c-lsf == 1][c-nb_channels == 1]);
@@ -151,6 +152,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
AVStream *st,
 mp3-frames = avio_rb32(s-pb);
 if (v  XING_FLAG_SIZE)
 mp3-header_filesize = avio_rb32(s-pb);
+min = FFMIN(fsize, mp3-header_filesize);
+delta = FFMAX(fsize, mp3-header_filesize) - min;
+if (fsize  mp3-header_filesize  0  delta  min  4)
+mp3-frames = 0;
 if (v  XING_FLAG_TOC)
 read_xing_toc(s, mp3-header_filesize, av_rescale_q(mp3-frames,
(AVRational){spf, c-sample_rate},
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]Ignore xing number of frames if filesize is wrong

2014-07-17 Thread Nicolas George 
Le nonidi 29 messidor, an CCXXII, Carl Eugen Hoyos a écrit :
  if (...  min - tolerance  2 * tolerance)
   ^^^   ^^^
This bit was a leftover from a slightly different version I tried at first,
it was wrong.

  I also find this version easier to understand.
 
 Thank you, new patch attached.
 
  I can not judge on the correctness, though.
 
 The logic is copied from asfdec.c.
 
 Please review, Carl Eugen

 diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c
 index 8335388..cf5aa73 100644
 --- a/libavformat/mp3dec.c
 +++ b/libavformat/mp3dec.c
 @@ -138,6 +138,7 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
 AVStream *st,
  
  MP3DecContext *mp3 = s-priv_data;
  static const int64_t xing_offtbl[2][2] = {{32, 17}, {17,9}};
 +uint64_t min, delta, fsize = avio_size(s-pb);
  
  /* Check for Xing / Info tag */
  avio_skip(s-pb, xing_offtbl[c-lsf == 1][c-nb_channels == 1]);
 @@ -151,6 +152,10 @@ static void mp3_parse_info_tag(AVFormatContext *s, 
 AVStream *st,
  mp3-frames = avio_rb32(s-pb);
  if (v  XING_FLAG_SIZE)
  mp3-header_filesize = avio_rb32(s-pb);
 +min = FFMIN(fsize, mp3-header_filesize);
 +delta = FFMAX(fsize, mp3-header_filesize) - min;
 +if (fsize  mp3-header_filesize  0  delta  min  4)
 +mp3-frames = 0;
  if (v  XING_FLAG_TOC)
  read_xing_toc(s, mp3-header_filesize, av_rescale_q(mp3-frames,
 (AVRational){spf, c-sample_rate},

No more remarks from me, but I do not maintain that part of the code.
Thanks.

Regards,

-- 
  Nicolas George


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