diff mbox

[FFmpeg-devel] avformat/mpl2dec: skip BOM when probing

Message ID 20170211105607.6115-1-onemda@gmail.com
State Superseded
Headers show

Commit Message

Paul B Mahol Feb. 11, 2017, 10:56 a.m. UTC 
Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavformat/mpl2dec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Carl Eugen Hoyos Feb. 11, 2017, 11:43 a.m. UTC | #1 
2017-02-11 11:56 GMT+01:00 Paul B Mahol <onemda@gmail.com>:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>

Please mention ticket #5442.

Thank you, Carl Eugen
Clément Bœsch Feb. 11, 2017, 5:44 p.m. UTC | #2 
On Sat, Feb 11, 2017 at 11:56:07AM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavformat/mpl2dec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
> index 59589d5..0e30cb0 100644
> --- a/libavformat/mpl2dec.c
> +++ b/libavformat/mpl2dec.c
> @@ -23,6 +23,8 @@
>   * MPL2 subtitles format demuxer
>   */
>  
> +#include "libavutil/intreadwrite.h"
> +
>  #include "avformat.h"
>  #include "internal.h"
>  #include "subtitles.h"
> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
>      const unsigned char *ptr = p->buf;
>      const unsigned char *ptr_end = ptr + p->buf_size;
>  
> +    if (AV_RB24(ptr) == 0xefbbbf)
> +        ptr += 3;
> +
>      for (i = 0; i < 2; i++) {
>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) != 3 &&
>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) != 2)
> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
>          if (!len)
>              break;
>  
> +        if (AV_RB24(p) == 0xefbbbf)
> +            p += 3;
> +

The BOM is supposed to be at the beginning of the file, not at every line.
The check should be outside the while loop.
Paul B Mahol Feb. 12, 2017, 11:51 a.m. UTC | #3 
On 2/11/17, Clement Boesch <u@pkh.me> wrote:
> On Sat, Feb 11, 2017 at 11:56:07AM +0100, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavformat/mpl2dec.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
>> index 59589d5..0e30cb0 100644
>> --- a/libavformat/mpl2dec.c
>> +++ b/libavformat/mpl2dec.c
>> @@ -23,6 +23,8 @@
>>   * MPL2 subtitles format demuxer
>>   */
>>
>> +#include "libavutil/intreadwrite.h"
>> +
>>  #include "avformat.h"
>>  #include "internal.h"
>>  #include "subtitles.h"
>> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
>>      const unsigned char *ptr = p->buf;
>>      const unsigned char *ptr_end = ptr + p->buf_size;
>>
>> +    if (AV_RB24(ptr) == 0xefbbbf)
>> +        ptr += 3;
>> +
>>      for (i = 0; i < 2; i++) {
>>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) !=
>> 3 &&
>>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) !=
>> 2)
>> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
>>          if (!len)
>>              break;
>>
>> +        if (AV_RB24(p) == 0xefbbbf)
>> +            p += 3;
>> +
>
> The BOM is supposed to be at the beginning of the file, not at every line.
> The check should be outside the while loop.
>
> --
> Clement B.
>

Removed check which was there, it appears to still work.
Clément Bœsch Feb. 12, 2017, 11:57 a.m. UTC | #4 
On Sun, Feb 12, 2017 at 12:51:11PM +0100, Paul B Mahol wrote:
> On 2/11/17, Clement Boesch <u@pkh.me> wrote:
> > On Sat, Feb 11, 2017 at 11:56:07AM +0100, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> >> ---
> >>  libavformat/mpl2dec.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
> >> index 59589d5..0e30cb0 100644
> >> --- a/libavformat/mpl2dec.c
> >> +++ b/libavformat/mpl2dec.c
> >> @@ -23,6 +23,8 @@
> >>   * MPL2 subtitles format demuxer
> >>   */
> >>
> >> +#include "libavutil/intreadwrite.h"
> >> +
> >>  #include "avformat.h"
> >>  #include "internal.h"
> >>  #include "subtitles.h"
> >> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
> >>      const unsigned char *ptr = p->buf;
> >>      const unsigned char *ptr_end = ptr + p->buf_size;
> >>
> >> +    if (AV_RB24(ptr) == 0xefbbbf)
> >> +        ptr += 3;
> >> +
> >>      for (i = 0; i < 2; i++) {
> >>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) !=
> >> 3 &&
> >>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) !=
> >> 2)
> >> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
> >>          if (!len)
> >>              break;
> >>
> >> +        if (AV_RB24(p) == 0xefbbbf)
> >> +            p += 3;
> >> +
> >
> > The BOM is supposed to be at the beginning of the file, not at every line.
> > The check should be outside the while loop.
> >
> > --
> > Clement B.
> >
> 
> Removed check which was there, it appears to still work.

Doesn't it skip the first event? Or maybe your libc skips it in sscanf?
Paul B Mahol Feb. 12, 2017, 12:10 p.m. UTC | #5 
On 2/12/17, Clement Boesch <u@pkh.me> wrote:
> On Sun, Feb 12, 2017 at 12:51:11PM +0100, Paul B Mahol wrote:
>> On 2/11/17, Clement Boesch <u@pkh.me> wrote:
>> > On Sat, Feb 11, 2017 at 11:56:07AM +0100, Paul B Mahol wrote:
>> >> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> >> ---
>> >>  libavformat/mpl2dec.c | 8 ++++++++
>> >>  1 file changed, 8 insertions(+)
>> >>
>> >> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
>> >> index 59589d5..0e30cb0 100644
>> >> --- a/libavformat/mpl2dec.c
>> >> +++ b/libavformat/mpl2dec.c
>> >> @@ -23,6 +23,8 @@
>> >>   * MPL2 subtitles format demuxer
>> >>   */
>> >>
>> >> +#include "libavutil/intreadwrite.h"
>> >> +
>> >>  #include "avformat.h"
>> >>  #include "internal.h"
>> >>  #include "subtitles.h"
>> >> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
>> >>      const unsigned char *ptr = p->buf;
>> >>      const unsigned char *ptr_end = ptr + p->buf_size;
>> >>
>> >> +    if (AV_RB24(ptr) == 0xefbbbf)
>> >> +        ptr += 3;
>> >> +
>> >>      for (i = 0; i < 2; i++) {
>> >>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c)
>> >> !=
>> >> 3 &&
>> >>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c)
>> >> !=
>> >> 2)
>> >> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
>> >>          if (!len)
>> >>              break;
>> >>
>> >> +        if (AV_RB24(p) == 0xefbbbf)
>> >> +            p += 3;
>> >> +
>> >
>> > The BOM is supposed to be at the beginning of the file, not at every
>> > line.
>> > The check should be outside the while loop.
>> >
>> > --
>> > Clement B.
>> >
>>
>> Removed check which was there, it appears to still work.
>
> Doesn't it skip the first event? Or maybe your libc skips it in sscanf?

Ah, first even is indeed skipped.
wm4 Feb. 15, 2017, 7:55 a.m. UTC | #6 
On Sat, 11 Feb 2017 11:56:07 +0100
Paul B Mahol <onemda@gmail.com> wrote:

> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavformat/mpl2dec.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
> index 59589d5..0e30cb0 100644
> --- a/libavformat/mpl2dec.c
> +++ b/libavformat/mpl2dec.c
> @@ -23,6 +23,8 @@
>   * MPL2 subtitles format demuxer
>   */
>  
> +#include "libavutil/intreadwrite.h"
> +
>  #include "avformat.h"
>  #include "internal.h"
>  #include "subtitles.h"
> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
>      const unsigned char *ptr = p->buf;
>      const unsigned char *ptr_end = ptr + p->buf_size;
>  
> +    if (AV_RB24(ptr) == 0xefbbbf)
> +        ptr += 3;
> +
>      for (i = 0; i < 2; i++) {
>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) != 3 &&
>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) != 2)
> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
>          if (!len)
>              break;
>  
> +        if (AV_RB24(p) == 0xefbbbf)
> +            p += 3;
> +
>          line[strcspn(line, "\r\n")] = 0;
>  
>          if (!read_ts(&p, &pts_start, &duration)) {

What happened to the equivalent patch I sent almost a year ago? My
patch skipped it only in the probe function, because the subtitle line
reader skips BOM already.

It referenced the same trac ticket.
Paul B Mahol Feb. 15, 2017, 9:07 a.m. UTC | #7 
On 2/15/17, wm4 <nfxjfg@googlemail.com> wrote:
> On Sat, 11 Feb 2017 11:56:07 +0100
> Paul B Mahol <onemda@gmail.com> wrote:
>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavformat/mpl2dec.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
>> index 59589d5..0e30cb0 100644
>> --- a/libavformat/mpl2dec.c
>> +++ b/libavformat/mpl2dec.c
>> @@ -23,6 +23,8 @@
>>   * MPL2 subtitles format demuxer
>>   */
>>
>> +#include "libavutil/intreadwrite.h"
>> +
>>  #include "avformat.h"
>>  #include "internal.h"
>>  #include "subtitles.h"
>> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
>>      const unsigned char *ptr = p->buf;
>>      const unsigned char *ptr_end = ptr + p->buf_size;
>>
>> +    if (AV_RB24(ptr) == 0xefbbbf)
>> +        ptr += 3;
>> +
>>      for (i = 0; i < 2; i++) {
>>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) !=
>> 3 &&
>>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) !=
>> 2)
>> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
>>          if (!len)
>>              break;
>>
>> +        if (AV_RB24(p) == 0xefbbbf)
>> +            p += 3;
>> +
>>          line[strcspn(line, "\r\n")] = 0;
>>
>>          if (!read_ts(&p, &pts_start, &duration)) {
>
> What happened to the equivalent patch I sent almost a year ago? My
> patch skipped it only in the probe function, because the subtitle line
> reader skips BOM already.

It doesn't appear that is true.

>
> It referenced the same trac ticket.

You needd to resend/ping patch more often.
wm4 Feb. 15, 2017, 9:26 a.m. UTC | #8 
On Wed, 15 Feb 2017 10:07:48 +0100
Paul B Mahol <onemda@gmail.com> wrote:

> On 2/15/17, wm4 <nfxjfg@googlemail.com> wrote:
> > On Sat, 11 Feb 2017 11:56:07 +0100
> > Paul B Mahol <onemda@gmail.com> wrote:
> >  
> >> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> >> ---
> >>  libavformat/mpl2dec.c | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
> >> index 59589d5..0e30cb0 100644
> >> --- a/libavformat/mpl2dec.c
> >> +++ b/libavformat/mpl2dec.c
> >> @@ -23,6 +23,8 @@
> >>   * MPL2 subtitles format demuxer
> >>   */
> >>
> >> +#include "libavutil/intreadwrite.h"
> >> +
> >>  #include "avformat.h"
> >>  #include "internal.h"
> >>  #include "subtitles.h"
> >> @@ -39,6 +41,9 @@ static int mpl2_probe(AVProbeData *p)
> >>      const unsigned char *ptr = p->buf;
> >>      const unsigned char *ptr_end = ptr + p->buf_size;
> >>
> >> +    if (AV_RB24(ptr) == 0xefbbbf)
> >> +        ptr += 3;
> >> +
> >>      for (i = 0; i < 2; i++) {
> >>          if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) !=
> >> 3 &&
> >>              sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) !=
> >> 2)
> >> @@ -94,6 +99,9 @@ static int mpl2_read_header(AVFormatContext *s)
> >>          if (!len)
> >>              break;
> >>
> >> +        if (AV_RB24(p) == 0xefbbbf)
> >> +            p += 3;
> >> +
> >>          line[strcspn(line, "\r\n")] = 0;
> >>
> >>          if (!read_ts(&p, &pts_start, &duration)) {  
> >
> > What happened to the equivalent patch I sent almost a year ago? My
> > patch skipped it only in the probe function, because the subtitle line
> > reader skips BOM already.  
> 
> It doesn't appear that is true.

Right, seems like it's not using FFTextReader.

> >
> > It referenced the same trac ticket.  
> 
> You needd to resend/ping patch more often.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

diff --git a/libavformat/mpl2dec.c b/libavformat/mpl2dec.c
index 59589d5..0e30cb0 100644
--- a/libavformat/mpl2dec.c
+++ b/libavformat/mpl2dec.c
@@ -23,6 +23,8 @@ 
  * MPL2 subtitles format demuxer
  */
 
+#include "libavutil/intreadwrite.h"
+
 #include "avformat.h"
 #include "internal.h"
 #include "subtitles.h"
@@ -39,6 +41,9 @@  static int mpl2_probe(AVProbeData *p)
     const unsigned char *ptr = p->buf;
     const unsigned char *ptr_end = ptr + p->buf_size;
 
+    if (AV_RB24(ptr) == 0xefbbbf)
+        ptr += 3;
+
     for (i = 0; i < 2; i++) {
         if (sscanf(ptr, "[%"SCNd64"][%"SCNd64"]%c", &start, &end, &c) != 3 &&
             sscanf(ptr, "[%"SCNd64"][]%c",          &start,       &c) != 2)
@@ -94,6 +99,9 @@  static int mpl2_read_header(AVFormatContext *s)
         if (!len)
             break;
 
+        if (AV_RB24(p) == 0xefbbbf)
+            p += 3;
+
         line[strcspn(line, "\r\n")] = 0;
 
         if (!read_ts(&p, &pts_start, &duration)) {