On 09-05-2019 01:15, Reimar Döffinger wrote:
> On Tue, May 07, 2019 at 10:05:12AM +0530, Shivam Goyal wrote:
>
>> +static int arecont_h264_probe(const AVProbeData *p)
>> +{
>> + int i, j, k, o = 0;
>> + int ret = h264_probe(p);
>> + const uint8_t id[] = {0x2D, 0x2D, 0x66, 0x62, 0x64, 0x72, 0x0D, 0x0A};
>
> Should be "static const" instead of just "const".
> Also this seems to be just "--fbdr\r\n"?
Okay, I would change it on next version, Yeah it is '--fbdr\r\n' The
file from Arecont h264 contains http header starting from this, many
times.
> + if (p->buf[i] == id[0] && !memcmp(id, p->buf + i, 8))
> + o++;
> Is that optimization really helping much?
> If you want to speed it up, I suspect it would be more
> useful to either go for AV_RL64 or memchr() or
> a combination.
> Also, sizeof() instead of hard-coding the 8.
>
> + if (o >= 1)
> + return ret + 1;
> Since you only check for >= 1 you could have aborted the
> scanning loop in the first match...
Yeah, this would be a great idea. I will change this.
> +static int read_raw_arecont_h264_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> + int ret, size, start, end, new_size = 0, i, j, k, w = 0;
> + const uint8_t id[] = {0x2D, 0x2D, 0x66, 0x62, 0x64, 0x72};
> "static", however this seems the same data (though 2 shorter).
> I'd suggest defining the signature just once.
>
> + uint8_t *data;
> + int64_t pos;
> +
> + //Extra to find the http header
> + size = 2 * ARECONT_H264_MIME_SIZE + RAW_PACKET_SIZE;
> + data = av_malloc(size);
> +
> + if (av_new_packet(pkt, size) < 0)
> + return AVERROR(ENOMEM);
> +
> + pkt->pos = avio_tell(s->pb);
> + pkt->stream_index = 0;
> + pos = avio_tell(s->pb);
> + ret = avio_read_partial(s->pb, data, size);
> + if (ret < 0) {
> + av_packet_unref(pkt);
> + return ret;
> + }
> + if (pos <= ARECONT_H264_MIME_SIZE) {
> + avio_seek(s->pb, 0, SEEK_SET);
> + start = pos;
> + } else {
> + avio_seek(s->pb, pos - ARECONT_H264_MIME_SIZE, SEEK_SET);
> + start = ARECONT_H264_MIME_SIZE;
> + }
> + ret = avio_read_partial(s->pb, data, size);
> + if (ret < 0 || start >= ret) {
> + av_packet_unref(pkt);
> + return ret;
> + }
> You need to document more what you are doing here.
> And even more importantly why you are using avio_read_partial.
> And why you allocate both a packet and a separate "data"
> with the same size.
> And why not use av_get_packet.
I saw the raw video demuxer, there it was using avio_read_partial.
Because it is allowed to read less data then specified.
> + if (i >= start && j + 1 > start && j + 1 <= end) {
> + memcpy(pkt->data + new_size, data + start, i - start);
> + new_size += i - start;
> + memcpy(pkt->data + new_size, data + j + 2, end - j - 1);
> + new_size += end - j - 1;
> + w = 1;
> + break;
> + } else if (i < start && j + 1 >= start && j + 1 < end) {
> + memcpy(pkt->data + new_size, data + j + 2, end - j -1);
> + new_size += end - j - 1;
> + w = 1;
> + break;
> + } else if (j + 1 > end && i > start && i <= end) {
> + memcpy(pkt->data + new_size, data + start, i - start);
> + new_size += i - start;
> + w = 1;
> + break;
> + }
> With some comments I might be able to review without
> spending a lot of time reverse-engineering this...
Okay, would add comments to this also.
I also have another idea to solve this findind and removing the http
header, but it would require to change RAW_PACKET_SIZE. So, just want to
know, can i change this? if yes, then how much can i change this?
Thanks for the review,
Shivam Goyal
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
