On Wed, Jul 25, 2012 at 11:00 PM, Martin Storsjö <[email protected]> wrote: > On Wed, 25 Jul 2012, Samuel Pitoiset wrote: > >> On Wed, Jul 25, 2012 at 8:05 PM, Martin Storsjö <[email protected]> wrote: >>> >>> On Sat, 21 Jul 2012, Samuel Pitoiset wrote: >>> >>>> --- >>>> libavformat/rtmpproto.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/libavformat/rtmpproto.c b/libavformat/rtmpproto.c >>>> index c003b37..5af03c4 100644 >>>> --- a/libavformat/rtmpproto.c >>>> +++ b/libavformat/rtmpproto.c >>>> @@ -859,11 +859,11 @@ static int handle_client_bw(URLContext *s, >>>> RTMPPacket *pkt) >>>> { >>>> RTMPContext *rt = s->priv_data; >>>> >>>> - if (pkt->data_size < 4) { >>>> + if (pkt->data_size != 5) { >>>> av_log(s, AV_LOG_ERROR, >>>> - "Client bandwidth report packet is less than 4 bytes >>>> long >>>> (%d)\n", >>>> + "Client bandwidth packet is not 5 bytes long (%d)\n", >>>> pkt->data_size); >>>> - return -1; >>>> + return AVERROR(EINVAL); >>>> } >>>> >>>> rt->client_report_size = AV_RB32(pkt->data); >>>> -- >>>> 1.7.11.1 >>> >>> >>> >>> This changes behaviour - is there a reason to treat it as an error if the >>> client bandwidth packet is longer than 5 bytes? I'd rather have the error >>> code change separate from the behaviour change, and an explanation why >>> you'd >>> like to change the behaviour. >> >> >> We already return an error code when a client bandwidth packet is less >> than 4 bytes long (ie. it's more or less the same behaviour for chunk >> size packets). So, I made these changes in order to be consistent >> regarding the other handle functions I have refactored. Otherwise, I >> submitted a separate patch which change the error code. > > > Ok, if the reason is to make things more consistent, that is a valid reason > I guess. If sent as a separate change from the other things with proper > explanation and motivation, I guess it might be ok,
Okay, I'll submit this change in a separate patch. > but then I'd rather have > the check in handle_chunk_size fixed instead - I'd rather be more liberal > when it comes to unknown data in packets. We already check the packet size in handle_chunk_size, so what do you mean here? > > Also, doesn't handle_ping and handle_server_bw need similar packet size > checks as well? Yes. -- Best regards, Samuel Pitoiset. _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
