On Tue, May 28, 2019 at 11:30:13PM +0200, Reimar Döffinger wrote: > Hi! > Did you intentionally not send to the list?
Okay I just realised I have been replying personally to all comments whereas I should have sent them to the list ':D > > On 28.05.2019, at 17:32, Swaraj Hota <swarajhota...@gmail.com> wrote: > > > On Sun, May 26, 2019 at 09:44:35PM +0200, Reimar Döffinger wrote: > >> On Sun, May 26, 2019 at 01:46:32AM +0530, Swaraj Hota wrote: > >>> + st = avformat_new_stream(s, NULL); > >>> + if (!st) > >>> + return AVERROR(ENOMEM); > >>> + > >>> + st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO; > >>> + st->codecpar->codec_id = AV_CODEC_ID_H264; > >>> + st->codecpar->width = ifv->width; > >>> + st->codecpar->height = ifv->height; > >>> + st->start_time = 0; > >>> + ifv->video_stream_index = st->index; > >> > >> I suspect that it would have been ok to > >> just assume/assert this will always be 0. > >> > > > > Okay, but I didn't get what exact change are you suggesting? Should > > I replace st->index with 0 here? Or should I get rid of > > ifv->video/audio_stream_index variables and instead use 0 and 1 > > everywhere? > > I think it's fine, now that you already wrote the code. > Just saying that you probably didn't really have to implement quite as much. > Will keep that in mind :) > >>> +static int ifv_read_packet(AVFormatContext *s, AVPacket *pkt) > >> > >> As far as I can tell, you just choose between audio > >> and video by the closest timestamp. > > > > Yes. > > > >> While this might be ok considering the limited importance > >> and uses of the format, it is rather simplistic. > >> I do not know if it follows the latest best practices, > >> but mov_find_next_sample is an example of a more > >> sophisticated way. > >> It takes into account whether the underlying transport > >> has issues seeking (like http, or even worse piped input) > >> and in that case prefers file position over timestamp > >> position. > >> It also avoids wasting time reading streams marked as > >> AVDISCARD_ALL (in mov_read_packet is that check). > >> Maybe others following the project more closely > >> can give additional/better best practice tips. > > > > Okay, I get that, but as you said considering the limited importance and > > uses of the format, we must also consider if it is worth all the effort, > > cuz it can also get more messy ':D Besides I have been working on this > > format since over a month or two now, and understanding, writing and > > debugging > > this change might take quite some time ':D > > > > I have been trying to comply with all good practices but I think a working > > demuxer is quite enough for a minority format like this (e.g. even the > > dhav format is quite simplistic), but again I can't say that, > > if others agree that it is a necessity I will work on it. > > Entirely depends on the purpose. > If the aim is to have a working demuxer, I think it is acceptable from what I > looked at. > I guess the only concern might be that not supporting piped input would be a > bit of a regression from the first patch. Is there another simpler way to support piped input? Can you explain a bit? > If the aim is to learn and possibly tackle more involved formats it might be > useful. But even then maybe rather as a separate patch later... > Actually another reason is that I now have to focus on my gsoc project, so yeah, a separate patch later seems like a good idea to me :) Thank you. Swaraj _______________________________________________ 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".
