diff mbox

[FFmpeg-devel,2/2] avformat/mxfdec: use the common packet pts setter function for opatom files

Message ID 20170907151140.616-2-cus@passwd.hu
State Accepted
Commit 01911b9b3cad85ff1c346165659c0181db661b3e
Headers show

Commit Message

Marton Balint Sept. 7, 2017, 3:11 p.m. UTC
Fixes ticket #6631.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mxfdec.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Comments

Michael Niedermayer Sept. 7, 2017, 10:37 p.m. UTC | #1
On Thu, Sep 07, 2017 at 05:11:40PM +0200, Marton Balint wrote:
> Fixes ticket #6631.
> 
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mxfdec.c | 12 +++---------
>  1 file changed, 3 insertions(+), 9 deletions(-)

please add a fate this if you push this

no more comments from me

thx

[...]
Marton Balint Sept. 13, 2017, 7:31 p.m. UTC | #2
On Fri, 8 Sep 2017, Michael Niedermayer wrote:

> On Thu, Sep 07, 2017 at 05:11:40PM +0200, Marton Balint wrote:
>> Fixes ticket #6631.
>>
>> Signed-off-by: Marton Balint <cus@passwd.hu>
>> ---
>>  libavformat/mxfdec.c | 12 +++---------
>>  1 file changed, 3 insertions(+), 9 deletions(-)
>
> please add a fate this if you push this

Unfortunately we only have a sample which is around 16 MB but it seems 
like an overkill to add such a big sample to the fate suite for such 
a minor feature. And I am also not very fond of truncating the file and 
adding truncated files to the fate suite... So unless somebody can provide 
an opAtom AVC intra file which is only a few frames long, I see no good 
way to fate test this. Do you have an idea how to proceed?

Thanks,
Marton
Tomas Härdin Sept. 14, 2017, 12:30 p.m. UTC | #3
On 2017-09-13 21:31, Marton Balint wrote:
>
> On Fri, 8 Sep 2017, Michael Niedermayer wrote:
>
>> On Thu, Sep 07, 2017 at 05:11:40PM +0200, Marton Balint wrote:
>>> Fixes ticket #6631.
>>>
>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>> ---
>>>  libavformat/mxfdec.c | 12 +++---------
>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>
>> please add a fate this if you push this
>
> Unfortunately we only have a sample which is around 16 MB but it seems 
> like an overkill to add such a big sample to the fate suite for such a 
> minor feature. And I am also not very fond of truncating the file and 
> adding truncated files to the fate suite... So unless somebody can 
> provide an opAtom AVC intra file which is only a few frames long, I 
> see no good way to fate test this. Do you have an idea how to proceed?

Try having the user that reported the bug generate a smaller + lower res 
sample?

/Tomas
Marton Balint Sept. 23, 2017, 5:26 p.m. UTC | #4
On Thu, 14 Sep 2017, Tomas Härdin wrote:

> On 2017-09-13 21:31, Marton Balint wrote:
>>
>> On Fri, 8 Sep 2017, Michael Niedermayer wrote:
>>
>>> On Thu, Sep 07, 2017 at 05:11:40PM +0200, Marton Balint wrote:
>>>> Fixes ticket #6631.
>>>>
>>>> Signed-off-by: Marton Balint <cus@passwd.hu>
>>>> ---
>>>>  libavformat/mxfdec.c | 12 +++---------
>>>>  1 file changed, 3 insertions(+), 9 deletions(-)
>>>
>>> please add a fate this if you push this
>>
>> Unfortunately we only have a sample which is around 16 MB but it seems 
>> like an overkill to add such a big sample to the fate suite for such a 
>> minor feature. And I am also not very fond of truncating the file and 
>> adding truncated files to the fate suite... So unless somebody can 
>> provide an opAtom AVC intra file which is only a few frames long, I 
>> see no good way to fate test this. Do you have an idea how to proceed?
>
> Try having the user that reported the bug generate a smaller + lower res 
> sample?

These are P2 camera files, the reporter can't provide smaller files.

Anyway I pushed the patch series as is, fate test can be added later if 
somebody comes up with a similar but smaller intra-frame OpAtom file.

Regards,
Marton
diff mbox

Patch

diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
index 3e2f5011ee..476d284c96 100644
--- a/libavformat/mxfdec.c
+++ b/libavformat/mxfdec.c
@@ -3217,15 +3217,9 @@  static int mxf_read_packet(AVFormatContext *s, AVPacket *pkt)
 
     pkt->stream_index = st->index;
 
-    if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && t->ptses &&
-        mxf->current_edit_unit >= 0 && mxf->current_edit_unit < t->nb_ptses) {
-        pkt->dts = mxf->current_edit_unit + t->first_dts;
-        pkt->pts = t->ptses[mxf->current_edit_unit];
-    } else if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
-        int ret = mxf_set_audio_pts(mxf, st->codecpar, pkt);
-        if (ret < 0)
-            return ret;
-    }
+    ret = mxf_set_pts(mxf, st, pkt, next_pos);
+    if (ret < 0)
+        return ret;
 
     mxf->current_edit_unit += edit_units;