Skip to content

MP3 code Withdrawn-- code from Arthur Taylor is better#493

Closed
jpffitch wants to merge 26 commits intolibsndfile:masterfrom
jpffitch:master
Closed

MP3 code Withdrawn-- code from Arthur Taylor is better#493
jpffitch wants to merge 26 commits intolibsndfile:masterfrom
jpffitch:master

Conversation

@jpffitch
Copy link
Copy Markdown

Think this does it. Not implemented writing by frames but that is really just copying code. Would appreciate any feedback

Copy link
Copy Markdown
Member

@erikd erikd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have write here, but also need read.

src/mp3.c Outdated
#include "common.h"
#include "mp3.h"

#if 0
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just remove that #if 0.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done; thank you for all your coments.

src/sndfile.c Outdated
{ SF_ERR_NO_ERROR , "No Error." },
{ SF_ERR_UNRECOGNISED_FORMAT , "Format not recognised." },
{ SF_ERR_SYSTEM , "System error." /* Often replaced. */ },
{ SF_ERR_SYSTEM , "System error." /* Often replaced. */ },
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??

src/sndfile.h.in Outdated
/* Endian-nes


options. */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment there should be unchanged.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry; my bad typing and I did not notice. fixed.

src/mp3.c Outdated


if (psf->file.mode == SFM_READ)
return SFE_UNIMPLEMENTED ;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will need to support read before this can be merged.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; that is my current focus. I still have multiple routes and do not know which way to go. I am still investigating. mpg123 seems heavy and foolswood's attempt seems to have stopped. But I have taken a copy of his code to investigate. minimp3 is wacky, being all one include file. May be the lightest way. There is code in LAME based on mpg123 but the documention/examples I have found look sparse.

return 1 ;
}

/* **** Note that if mode is MONO (3) then left and right channels are averaged and the data is changed. Hence the _M functions **** */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This stereo mix down to mono functionality should probably be dropped.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you suggest mono is signalled only by sf.channels? That still leaves the two types of stereo to resolve. I do need to ensure sf.channels is 1 or 2 only.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So you suggest mono is signalled only by sf.channels?

Yes.

I do need to ensure sf.channels is 1 or 2 only.

A number of formats already require things like that. See sf_format_check in src/sndfile.c.

src/mp3.c Outdated

#define CHUNK_SIZE (512)
#define FRAME_SIZE (1152)
#define NORMALISE (0.000030517578125)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is that magic number all about?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1/32K to change short range to [-1, =1]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code now removed.

#include "lame.h"
#include "mpg123.h"
#include <lame/lame.h>
#include <mpg123.h>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@njh njh mentioned this pull request Nov 20, 2019
@jpffitch jpffitch changed the title MP3 outout using LAME MP3 code Withdrawn-- code from Arthur Taylor is better Nov 25, 2019
@jpffitch jpffitch closed this Nov 25, 2019
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.

2 participants