All of lore.kernel.org
 help / color / mirror / Atom feed
From: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
To: Vinod Koul <vinod.koul@intel.com>
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>,
	patches@opensource.wolfsonmicro.com,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
	alsa-devel@alsa-project.org
Subject: Re: [PATCH] crec: Add option to specify codec ID
Date: Wed, 23 Nov 2016 10:21:58 +0000	[thread overview]
Message-ID: <1479896518.4317.7.camel@rf-debian.wolfsonmicro.main> (raw)
In-Reply-To: <20161123034103.GZ2698@localhost>

On Wed, 2016-11-23 at 09:11 +0530, Vinod Koul wrote:
> On Fri, Nov 18, 2016 at 04:17:37PM +0000, Charles Keepax wrote:
> > On Fri, Nov 18, 2016 at 08:39:38AM -0600, Pierre-Louis Bossart wrote:
> > > 
> > > >>If you're debugging you may not care about the file format, you're
> > > >>actually interested in the raw data you get from the codec so dumping
> > > >>the output to a raw file would be useful even if you can't load that
> > > >>file into a music player.
> > > >
> > > >While I agree with you on this, am worried that adding codecs may make
> > > >people think that we can record mp3 file for exmaple, which is not the case.
> > > >
> > > >I think we can add pcm and bespoke as formats supported and allow any format to
> > > >be dumped to stdio. That way it is pretty clear to people ...
> > > 
> > > Crec as is it is pretty useless with PCM only...If we added the profile
> > > selection and things like bitrate information it'd be straightforward to
> > > support elementary bitstreams like MP3 or AAC ADTS, you just dump the data
> > > to a file. Things that require a header or MP4 integration would require
> > > additional libraries, this is no longer 'tiny'.
> > 
> > What about this as a suggestion, if we remove the code that adds
> > the WAV header. Then all the output from crecord is raw data and
> > the addition of any headers or additional wrapping is up to the
> > user. That keeps crecord 'tiny' and allows us to support all the
> > formats in a consistent way so no one gets confused.
> 
> Naah, crecord is a utility. If you need to do above you have tinycompress
> APIs to get the data and pack it the way you like.. You don't and shouldn't
> use crecord for that.
> 

But you can't call APIs from a shell command line, it needs a tool. For
debugging and testing we need a utility that can extract the raw data
from a codec. It's not the case that you _must_ have this data wrapped
in a file format just because normally it would be - for debugging the
raw dump may be sufficient, you aren't necessarily only interested in
playing it in a music app, for debugging you are likely to be more
interested in the actual bytes and in fact it could be preferable to
have it raw and not modified into a container file.

> > We haven't actually used the WAV header stuff since the very
> > early versions of our firmware that didn't use compression on the
> > stream and actually did output PCM data and I don't think anyone
> > else has ever used the compressed interface for PCM.
> 
> Thats fine by me. The only reason we have a WAV header is that we can pack
> PCM files, for rest of the formats it become tricky as Pierre mention so lets
> not go that road :)
> 

I'm confused now about what you want.
If we don't want to allow raw dumps in crecord we need another tool -
say cdump - that can dump raw, but obviously it would be 99% identical
to crecord except that it's blessed with the ability to dump raw, and I
don't see the point of having two near-identical tools.

The alternative is that Cirrus maintains its own branched version of
tinycompress with useful tools but that doesn't seem like a sensible
road to go down either.

  reply	other threads:[~2016-11-23 10:22 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-16 11:44 [PATCH] crec: Add option to specify codec ID Richard Fitzgerald
2016-11-16 13:05 ` Vinod Koul
2016-11-16 13:07   ` Richard Fitzgerald
2016-11-16 13:48     ` Charles Keepax
2016-11-16 14:53       ` Richard Fitzgerald
2016-11-18  3:53         ` Vinod Koul
2016-11-18 10:11           ` Richard Fitzgerald
2016-11-18 10:29             ` Vinod Koul
2016-11-18 14:39               ` Pierre-Louis Bossart
2016-11-18 16:17                 ` Charles Keepax
2016-11-23  3:41                   ` Vinod Koul
2016-11-23 10:21                     ` Richard Fitzgerald [this message]
2016-11-23 10:38                       ` Vinod Koul
2016-11-27 17:52                         ` Pierre-Louis Bossart
2016-11-28  9:47                           ` Richard Fitzgerald
2016-11-28 15:54                             ` Vinod Koul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1479896518.4317.7.camel@rf-debian.wolfsonmicro.main \
    --to=rf@opensource.wolfsonmicro.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=ckeepax@opensource.wolfsonmicro.com \
    --cc=patches@opensource.wolfsonmicro.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.