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

Change pCodecParameters to pLocalCodecParameters in README.md #37

Open
Page-David opened this issue Feb 24, 2019 · 2 comments
Open

Change pCodecParameters to pLocalCodecParameters in README.md #37

Page-David opened this issue Feb 24, 2019 · 2 comments

Comments

@Page-David
Copy link

Page-David commented Feb 24, 2019

Hi, I noticed this line of code in README.md

printf("\tCodec %s ID %d bit_rate %lld", pLocalCodec->long_name, pLocalCodec->id, pCodecParameters->bit_rate);

in which pLocalCodec is not declared in the article before used which might cause confusion in my view. Digging into the c code, I found this line.

pCodecParameters = pLocalCodecParameters;

As for a guess, maybe the author firstly write the c code and copy-and-paste these important parts into README and lead to this issue. By the way, I think adding \n afterward printfs could make output looks better. Hope that useful.

UPDATE
This line of code has the same problem:

AVCodecContext *pCodecContext = avcodec_alloc_context3(pCodec);

Maybe it will be better to replace pCodec with pLocalCodec.

@leandromoreira
Copy link
Owner

Good catch.

@Page-David
Copy link
Author

Page-David commented Feb 27, 2019

Hi, it is now that I finally find that simply replacing pLocalCodec with pLocalCodecParameters might not be a perfect solution. Instead, it might be better to firstly state that when we do the while loop to read AVPacket we need to distinguish video track from sound track. Also, when we create pCodecContext and call avcodec_parameters_to_context, we must use AVCodec and AVCodecParameters from the video track. Thus we need to "remember" them with pCodec etc. What do you think about?

Btw, could you please explain why do we need avformat_alloc_context() for *pFormatContext and why do we need av_frame_unref(pFrame);? So far, assigning NULL to *pFormatContext or remove av_frame_unref did not cause any problem for me. In chapter 2, AVFormatContext was assigned like AVFormatContext *input_format_context = NULL; why this time not avformat_alloc_context()?

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

No branches or pull requests

2 participants