[FFmpeg-devel] [PATCH] avformat/dashdec: fix memleak for commit commit e134c203

Steven Liu lq at chinaffmpeg.org
Fri Mar 20 12:44:11 EET 2020



> 2020年3月20日 下午4:34,Anton Khirnov <anton at khirnov.net> 写道:
> 
> Quoting Steven Liu (2020-03-19 00:30:52)
>> 
>> Thanks
>> 
>> Steven Liu
>> 
>>> 2020年3月19日 上午12:48,Andreas Rheinhardt <andreas.rheinhardt at gmail.com> 写道:
>>> 
>>> Hello,
>>> 
>>> before the actual review I want to tell you that I actually agree with
>>> Nicholas. I don't see the point of already parsing stuff that is not
>>> used yet, especially if it involves allocations and checks.
>> 
>> Yes, I agreed with all of you, but I think we should try to "full
>> support all the attributes parse" what writes in the sepcification,
>> And try to use all the attribute. Because those attribute in the
>> specification is not useless and have some reason, I’m not member of
>> the specification author,
> 
> You seem to be contradicting yourself. Either you agree that it is
> pointless to parse attributes that do not get used, in which case you
> shouldn't send patches that do precisely that.
> Or you believe that we should in fact parse things that do not get used,
> then you do not agree with the other commenters. Both at once are not
> possible.
> 
> FWIW I believe that the entire point of demuxers is to export useful
> data to the caller. Therefore any code that does not contribute towards
> that goal is useless and should not be present in the tree.
Why the specification add these attributes into the documents if these attributes not useful, just kidding?
I said I’m not the specification author.
But I see the specification said: for example:
 
@minFrameRate

O

specifies the minimum @framerate value in all Representations in this Adaptation Set. This value is encoded in the same format as the @frameRate attribute.

If not present, the value is unknown.


Then, I think demuxer should give user warning message if the result is not same as specification, because it maybe will get wrong result if continue.
I agree all of you is the precondition, because I need friendly comments, but I think I need think what you guys said, and think about how to make the demuxer better,
Not just say something and don’t care about it, I add these attributes is just want make dashdec more standard support, not just partial support.

Review patch welcome.

> 
> -- 
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".

Thanks

Steven Liu





More information about the ffmpeg-devel mailing list