Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve avilib code #1218

Closed
wants to merge 3 commits into from
Closed

Improve avilib code #1218

wants to merge 3 commits into from

Conversation

s0m30n3
Copy link

@s0m30n3 s0m30n3 commented Mar 19, 2019

For a project of mine, I create an AVI file using my own code.
I'm already using libgpac to create MP4 files so only using libgpac would be quite logic.
The only feature missing in libgpac is adding an AVI subtitle.
Eventually I'll add it but I'm starting with some small commits to minimize regression risk.

Currently, we allocate buffers, we fill data and then we write the full chunk on disk.
I propose to write chunk fields one after another to avoid allocating/freeing memory for each chunk.
I created some helpers and demonstrated their usage reimplementing a function.
If you don't like "if(ret) goto on_error" lines, we can use a macro like CHECK_GOTO(conditon, label).

My ultimate goal is to store all needed informations in avi_t and write the AVI file once for all.
It'll use more memory so I'm not sure whether it's an improvement.

Anyway, I'm open to suggestions.

@jeanlf
Copy link
Member

jeanlf commented Jun 29, 2019

Thanks for looking into this. I doubt storing the entire avi in memory is a good idea. However, using an internal buffer to serialize chunks rather than using file io would be indeed a great improvement, especially for GPAC 0.9.0: for now avi muxer is a sink filter, such a change would make it a true mux filter with redirectable output. Do you think you could try patching the avi writer to write to internal mem and notify the caller through callbacks?

@codecov-io
Copy link

Codecov Report

Merging #1218 into master will increase coverage by 0.51%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1218      +/-   ##
==========================================
+ Coverage   49.79%   50.31%   +0.51%     
==========================================
  Files         323      323              
  Lines      228164   227427     -737     
==========================================
+ Hits       113616   114422     +806     
+ Misses     114548   113005    -1543
Impacted Files Coverage Δ
src/media_tools/avilib.c 43.06% <0%> (-1.89%) ⬇️
src/ietf/sdp.c 0% <0%> (-66.73%) ⬇️
modules/rtp_in/rtp_stream.c 0% <0%> (-47.32%) ⬇️
src/ietf/rtp.c 32.3% <0%> (-35.61%) ⬇️
modules/rtp_in/sdp_load.c 0% <0%> (-33.85%) ⬇️
src/ietf/rtcp.c 29.85% <0%> (-27.02%) ⬇️
modules/rtp_in/rtp_in.c 10.8% <0%> (-23.38%) ⬇️
modules/rtp_in/sdp_fetch.c 0% <0%> (-20.24%) ⬇️
src/ietf/rtp_depacketizer.c 0% <0%> (-16.33%) ⬇️
modules/rtp_in/rtp_session.c 0% <0%> (-10.21%) ⬇️
... and 101 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe8de95...c12f670. Read the comment docs.

@jeanlf
Copy link
Member

jeanlf commented Sep 29, 2022

no progress on this and not really the approach we would need, closing.

@jeanlf jeanlf closed this Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants